Quoting Nicolas George (2023-11-17 10:44:22) > Anton Khirnov (12023-11-09): > > I am obviously not proposing that, given the amount of patches I sent so > > far to keep sub2video working. > > Ok, sorry. > > > > > > I can suggest this: have demuxer code emit virtual subtitles packets to > > > trigger the sending of the heartbeat frames. > > > > That's what already happens, unless I misunderstand what you mean. > > Right now, the heartbeat code path is: > > demuxer → heartbeat → filter > > But the normal frame code path is: > > demuxer → decode frame → filter > > And it causes a problem because decode frame is in a different thread, > so there is a race condition between the heartbeat frames and the normal > frames.
There is no race in current master, because the decoding thread only runs while the main thread is waiting, and vice versa. That changes after this patchet of course, but... > Instead, if you make the demuxer generate a dummy packet instead of a > heartbeat frame, you get these code paths: > > demuxer → real sub packet → decode frame → filter > demuxer → dummy hb packet → decode frame → filter > > There is no race condition between the real sub packet and the dummy hb > packet because they both happen in the demuxer thread, and after that > they follow the same code path so there is no race condition either. ...that is exactly what I'm doing in 19-20/24. > > > > Another possibility is to make the call independently of the state of > > the graph, like this > > --- a/fftools/ffmpeg_filter.c > > +++ b/fftools/ffmpeg_filter.c > > @@ -2274,8 +2274,7 @@ void ifilter_sub2video_heartbeat(InputFilter > > *ifilter, int64_t pts, AVRational t > > or if we need to initialize the system, update the > > overlayed subpicture and its start/end times */ > > sub2video_update(ifp, pts2 + 1, NULL); > > - > > - if (av_buffersrc_get_nb_failed_requests(ifp->filter)) > > + else > > sub2video_push_ref(ifp, pts2); > > } > > Sending frames when they are not wanted can have bad consequences. I do > not have the time to look for a test case right now, but I have already > delayed replying to this mail too much. > > I think a situation where subtitles are delayed with the setpts filter > would trigger a problem with this change: the heartbeat frames will > accumulate in the input of the overlay filter while without this patch > only the real subtitles frames accumulate. For a medium delay, normal > frames would be in the dozens while heartbeat frames would be in the > thousands. I do not hink there should be any excessive buffering, because the scheduling code will simply stop reading from that input and only read from the lagging one until it catches up. > > This actually seems to improve filter-overlay-dvdsub-2397, where > > the first subtitle appears two frames later, which more closely matches > > the subtitles timestamp stored in the file. > > I would need to look into it. Does it improve it with your changes or > also with the precedent non-threaded code? The improvement happens immediately when the change is made, i.e. without threading. With threading it remains the same. -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
