GNOME Bugzilla – Bug 661726
subparse: missing race condition protect with seek
Last modified: 2016-04-18 14:37:00 UTC
This is what I observed when exercise srt subtitle on my embedded system. But I thought it should also happen in pc. The sympton is that: frequently do forward seek and backward seek sometimes, the subtititle will not be displayed after one backward seek. And it will display again after the video comes to the last playback position before this seek. And with some debug, I found there the issue is due to missing race condition protection: .. the self->segment will be written after push event_new_seek in gst_sub_parse_src_event(). This will happen after FLUSH_STOP. .. the self->segment also will be written by handle_buffer()/gst_sub_parse_chain(). This also will happen after FLUSH_STOP. So it will have un-certain value of self->segment when push new segement event to downstream in gst_sub_parse_chain(). ================================================================= To be more precise, the expected result is that after seek, the seek segment should be set before the chain process the 1st buffer after seek. But this is not the case since missing race condition protection. And I will attached my fix here for your review.
Created attachment 198985 [details] [review] Bugzilla_661726_1.patch - patched gstsubparse.h to add some variable to protect the race condition
Created attachment 198986 [details] [review] Bugzilla_661726_2.patch - patch gstsubparse.c This is only take flush seek in consideration so set is_seek_requested flag in case of GST_EVENT_FLUSH_STOP. how to fix in case of Seek without flush needs your advice.
Is this still a problem with 1.0? FLUSH_STOP and chain are supposed to be called from exactly the same thread, so there is no race condition involved here unless something else goes completely wrong already. Do you have a way to reproduce this somehow?
Comment on attachment 198986 [details] [review] Bugzilla_661726_2.patch - patch gstsubparse.c Please update to latest git master, don't use C99 comments and use the GStreamer coding style (there's a script in gstreamer/tools/gst-indent). Also please attach in "git format-patch" style format :)
I still see this happening in 1.4: seek to 0ms from start subparse: <parser:src> Handling qos event subparse: <parser:src> Handling seek event subparse: <parser:sink> Handling flush-start event subparse: <parser> flow: flushing subparse: <parser:sink> Handling flush-stop event subparse: <parser:sink> Handling custom-downstream-sticky event streamsynchroni: <streamsynchronizer1:sink_2> Non-TIME segment: bytes subparse: <parser:sink> Handling segment event subparse: <parser> newsegment (time) subparse: discontinuity subparse: initialising parser subparse: <parser> successfully converted 4096 characters from ISO-8859-15 to UTF-8 subparse: <parser> need_segment=0 subparse: <parser> Parsing line '1' subparse: <parser> Parsing line '00:00:12,821 --> 00:00:16,229' subparse: parsing timestamp '00:00:12,821' subparse: parsing timestamp '00:00:16,229' subparse: <parser> Parsing line 'Nous avons trouvés ça dans les' subparse: <parser> Parsing line 'affaires de Mlle Reed.' subparse: <parser> Parsing line '' subparse: <parser> Parsing line '2' subparse: <parser> Parsing line '00:00:16,354 --> 00:00:18,263' subparse: parsing timestamp '00:00:16,354' subparse: parsing timestamp '00:00:18,263' subparse: <parser> Parsing line 'C'est une carte d'accés pour l'ONU.' subparse: <parser> Parsing line '' subparse: <parser> Parsing line '3' subparse: <parser> Parsing line '00:00:18,388 --> 00:00:20,730' subparse: parsing timestamp '00:00:18,388' subparse: parsing timestamp '00:00:20,730' subparse: <parser> segment after seek: time segment start=0:00:00.000000000, offset=0:00:00.000000000, stop=99:99:99.999999999, rate=1.000000, applied_rate=1.000000, flags=0x01, time=0:00:00.000000000, base=0:00:00.000000000, position 0:00:00.000000000, duration 99:99:99.999999999 subparse: <parser> Parsing line '- A quoi ?' subparse: <parser> set need_segment=1 subparse: <parser> Parsing line '- Au Président Hassan et sa famille.' subparse: <parser> Parsing line '' subparse: <parser> Sending text '- A quoi ? - Au Président Hassan et sa famille.', 0:00:18.388000000 + 0:00:02.342000000 GStreamer-WARNING **: ../../gitsrc/gst/gstpad.c:4042:gst_pad_push_data:<parser:src> Got data flow before segment event The "set need_segment=1" print should happen before parsing starts in the chain function. Could we just send the segment event directly after sending the seek event ?
Can you provide a testcase for this? But not sure why it doesn't just send the SEGMENT event when it receives one or on the first buffer
Seems to be same as bug #747498, but the other one has more up-to-date patches. *** This bug has been marked as a duplicate of bug 747498 ***