GNOME Bugzilla – Bug 659485
mpegpsdemux: large MPEG PS file not working when seeking/transcoding with encodebin
Last modified: 2011-09-30 14:30:01 UTC
Got a 2 GB MPEG PS file I am trying to transcode. Edward looked into it and it seems to be a bug in MPEGPSDEMUX. <bilboed> cschalle, can't spend more time on it cschalle, but if it works in push mode, and it works with smaller files ... there's just something wrong in the PCR<=>offset conversion code
Created attachment 197156 [details] [review] mpegpsdemux: take into account SCR offset when seeking Since the seeking byte offset is chosen by linear interpolation from SCR values, we need to take that first SCR into account to end up near the correct offset. Otherwise, as the code does a linear search after that first seek, it will take a LOOOOOONG time to get there for streams which don't start at zero.
Created attachment 197171 [details] [review] mpegdemux: answer position query with a stream time position
Created attachment 197172 [details] [review] mpegpsdemux: remove unused field
Thanks for these patches Vincent, I think the position one works now, however I also need it to return the correct value for query_duration in stream time to be able to do the progress calculation, my assumption is that the query_duration call has the same problem as the query_position one you fixed.
Ok, so after talking to Vincent on IRC seems the duration query does give the right response, but the position one might not. When adding some debug print statements to Transmageddon I get the following output when starting to transcode: position is 1803826766666 duration is 1785296000000 value is 1.01037966066 So it seems to report a position higher then the duration when position in the beginning should be close to 0.
Created attachment 197470 [details] [review] baseparse: position reporting should be in stream time
Culprit was baseparse this time, not taking segment start into account. Progress in Transmageddon now works with this particular file.
Created attachment 197474 [details] [review] baseparse: position reporting should be in stream time Use gst_segment_to_stream_time instead of offseting manually, as suggested by slomo.
Created attachment 197475 [details] [review] baseparse: position reporting should be in stream time Let the demuxer have first say as well.
Tested and verified 470 patch to be working, hope this patch can get merged soon. As a bonus in addition to the progress bar in Transmageddon fix, this also fixes issue where Nautilus was not thumbnailing these movies.
Review of attachment 197474 [details] [review]: Looks good in general ::: libs/gst/base/gstbaseparse.c @@ +3156,3 @@ GST_CLOCK_TIME_IS_VALID (parse->segment.last_stop)) { + gst_segment_to_stream_time (&parse->segment, parse->segment.format, + parse->segment.last_stop); dest_value = ...
Review of attachment 197475 [details] [review]: Good in general but: ::: libs/gst/base/gstbaseparse.c @@ +3160,3 @@ + GST_CLOCK_TIME_IS_VALID (parse->segment.last_stop)) { + gst_segment_to_stream_time (&parse->segment, parse->segment.format, + parse->segment.last_stop); dest_value = ...
Created attachment 197536 [details] [review] baseparse: position reporting should be in stream time Let the demuxer have first say as well.
Ha, er, embarassing...
Comment on attachment 197171 [details] [review] mpegdemux: answer position query with a stream time position Committed with changes, thanks.