| Message ID | 20190206235740.6357-1-jeebjp@gmail.com |
|---|---|
| State | New |
| Headers | show |
2019-02-07 0:57 GMT+01:00, Jan Ekström <jeebjp@gmail.com>: > From: Masaki Tanaka <maki.rxrz@gmail.com> > > Seems to fix mistaken cases of discontinuity handling in MPEG-TS > when program structure changes. > > Additional changes to patch from its original state by Jan Ekström. > --- > > Had been meaning to post this for comments/discussion for a while, as > this seems to make timestamps continuously rising in at least one of > my samples where the program structure changes mid-stream. > > The original version of this patch can be found at: > https://github.com/jeeb/ffmpeg/commit/6117366eaadbaf48bbd88eb2a353dfc852ff3400 > > The sample for which this helped is available at: > https://kuroko.fushizen.eu/samples/pid_switch_sample.ts > > The difference of timestamps received from libavformat can be seen with: > https://kuroko.fushizen.eu/screenshots/ffmpeg/blue_vanilla-orange_patched.png Am I correct that even with this patch, the sample cannot be decoded / played correctly with FFmpeg / FFplay? No opinion about the patch, just curious, Carl Eugen
On Thu, Feb 7, 2019, 02:22 Carl Eugen Hoyos <ceffmpeg@gmail.com wrote: > 2019-02-07 0:57 GMT+01:00, Jan Ekström <jeebjp@gmail.com>: > > From: Masaki Tanaka <maki.rxrz@gmail.com> > > > > Seems to fix mistaken cases of discontinuity handling in MPEG-TS > > when program structure changes. > > > > Additional changes to patch from its original state by Jan Ekström. > > --- > > > > Had been meaning to post this for comments/discussion for a while, as > > this seems to make timestamps continuously rising in at least one of > > my samples where the program structure changes mid-stream. > > > > The original version of this patch can be found at: > > > https://github.com/jeeb/ffmpeg/commit/6117366eaadbaf48bbd88eb2a353dfc852ff3400 > > > > The sample for which this helped is available at: > > https://kuroko.fushizen.eu/samples/pid_switch_sample.ts > > > > The difference of timestamps received from libavformat can be seen with: > > > https://kuroko.fushizen.eu/screenshots/ffmpeg/blue_vanilla-orange_patched.png > > Am I correct that even with this patch, the sample cannot > be decoded / played correctly with FFmpeg / FFplay? > > No opinion about the patch, just curious, Carl Eugen > Yes, ffmpeg.c at least is static in its stream selection (and probably so is ffplay.c) so they would ignore the other video stream either before or after the switch depending on how much you probe and if you do manual stream selection. That said, this is a simple sample (no alternatives, just a clear cut switch) so -merge_pmt_versions 1 should work for it to get both the sd and hd parts decoded. This attempts to re-use a previous AVStream when a stream is removed. We do have the mechanism in mpegts.c to signal that a program's structure has changed, but unfortunately it has not been utilized in ffmpeg.c. Jan >
2019-02-07 8:18 GMT+01:00, Jan Ekström <jeebjp@gmail.com>: > On Thu, Feb 7, 2019, 02:22 Carl Eugen Hoyos <ceffmpeg@gmail.com wrote: > >> 2019-02-07 0:57 GMT+01:00, Jan Ekström <jeebjp@gmail.com>: >> > From: Masaki Tanaka <maki.rxrz@gmail.com> >> > >> > Seems to fix mistaken cases of discontinuity handling in MPEG-TS >> > when program structure changes. >> > >> > Additional changes to patch from its original state by Jan Ekström. >> > --- >> > >> > Had been meaning to post this for comments/discussion for a while, as >> > this seems to make timestamps continuously rising in at least one of >> > my samples where the program structure changes mid-stream. >> > >> > The original version of this patch can be found at: >> > >> https://github.com/jeeb/ffmpeg/commit/6117366eaadbaf48bbd88eb2a353dfc852ff3400 >> > >> > The sample for which this helped is available at: >> > https://kuroko.fushizen.eu/samples/pid_switch_sample.ts >> > >> > The difference of timestamps received from libavformat can be seen with: >> > >> https://kuroko.fushizen.eu/screenshots/ffmpeg/blue_vanilla-orange_patched.png >> >> Am I correct that even with this patch, the sample cannot >> be decoded / played correctly with FFmpeg / FFplay? >> >> No opinion about the patch, just curious, Carl Eugen >> > > Yes, ffmpeg.c at least is static in its stream selection (and probably so > is ffplay.c) so they would ignore the other video stream either before or > after the switch depending on how much you probe and if you do manual > stream selection. > > That said, this is a simple sample (no alternatives, just a clear cut > switch) so -merge_pmt_versions 1 should work for it to get both the sd and > hd parts decoded. This attempts to re-use a previous AVStream when a stream > is removed. Thank you, I forgot about this switch: The original timestamp issue is not reproducible with -merge_pmt_versions, right? Carl Eugen (who hopes you can use fieldmatch to watch this...)
On Thu, Feb 7, 2019 at 3:03 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2019-02-07 8:18 GMT+01:00, Jan Ekström <jeebjp@gmail.com>: > > On Thu, Feb 7, 2019, 02:22 Carl Eugen Hoyos <ceffmpeg@gmail.com wrote: > > > >> 2019-02-07 0:57 GMT+01:00, Jan Ekström <jeebjp@gmail.com>: > >> > From: Masaki Tanaka <maki.rxrz@gmail.com> > >> > > >> > Seems to fix mistaken cases of discontinuity handling in MPEG-TS > >> > when program structure changes. > >> > > >> > Additional changes to patch from its original state by Jan Ekström. > >> > --- > >> > > >> > Had been meaning to post this for comments/discussion for a while, as > >> > this seems to make timestamps continuously rising in at least one of > >> > my samples where the program structure changes mid-stream. > >> > > >> > The original version of this patch can be found at: > >> > > >> https://github.com/jeeb/ffmpeg/commit/6117366eaadbaf48bbd88eb2a353dfc852ff3400 > >> > > >> > The sample for which this helped is available at: > >> > https://kuroko.fushizen.eu/samples/pid_switch_sample.ts > >> > > >> > The difference of timestamps received from libavformat can be seen with: > >> > > >> https://kuroko.fushizen.eu/screenshots/ffmpeg/blue_vanilla-orange_patched.png > >> > >> Am I correct that even with this patch, the sample cannot > >> be decoded / played correctly with FFmpeg / FFplay? > >> > >> No opinion about the patch, just curious, Carl Eugen > >> > > > > Yes, ffmpeg.c at least is static in its stream selection (and probably so > > is ffplay.c) so they would ignore the other video stream either before or > > after the switch depending on how much you probe and if you do manual > > stream selection. > > > > That said, this is a simple sample (no alternatives, just a clear cut > > switch) so -merge_pmt_versions 1 should work for it to get both the sd and > > hd parts decoded. This attempts to re-use a previous AVStream when a stream > > is removed. > > Thank you, I forgot about this switch: The original timestamp issue > is not reproducible with -merge_pmt_versions, right? > Just replicated my tests, and yes you are correct. The change seems to happen because AVPrograms get updated as the programs change, but current wrap reference will not get updated accordingly. The patch seems to help but I am not 100% sure of the logic (given that taking in both very high and low values makes it possible to pick a different reference depending on the order of the streams within the AVProgram). If someone's interested in how I checked (PID 0x121 is the one that pops up first and thus merge_pmt_versions will only export it): 1. vanilla, with AVStream merging: ffprobe pid_switch_sample.ts -select_streams "0#0x121" -show_frames -of json -merge_pmt_versions 1 > 121_track_merged.json 2. vanilla, without AVStream merging: ffprobe pid_switch_sample.ts -select_streams "0#0x121" -show_frames -of json > 121_track.json 3. see the difference with a plotting script located @ http://up-cat.net/p/0f8b428a pts_plotter.py 121_track.json 121_track_merged.json 4. patched, same command as step 2 as 121_track_patched.json or so 5. see the difference with the plotting script pts_plotter.py 121_track.json 121_track_patched.json You can also do the same with PID 111 (the HD PID) with regards to vanilla VS patched. Results are rather similar. > Carl Eugen > (who hopes you can use fieldmatch to watch this...) fieldmatch,decimate should be OK for the show itself, but generally I am lazy and use yadif for non-encoding (viewing) cases. That makes sure that all the useless advertising that often is fully interlaced will also be correctly shown. Jan
2019-02-07 20:26 GMT+01:00, Jan Ekström <jeebjp@gmail.com>: > On Thu, Feb 7, 2019 at 3:03 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> Carl Eugen >> (who hopes you can use fieldmatch to watch this...) > > fieldmatch,decimate should be OK for the show itself, but generally I decimate seems unneeded for this specific sample. > am lazy and use yadif for non-encoding (viewing) cases. That makes > sure that all the useless advertising that often is fully interlaced > will also be correctly shown. Of course! Carl Eugen
diff --git a/libavformat/utils.c b/libavformat/utils.c index 7afef545fe..0efb1aae9c 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -815,8 +815,41 @@ static int update_wrap_reference(AVFormatContext *s, AVStream *st, int stream_in while (program) { if (program->pts_wrap_reference != pts_wrap_reference) { for (i = 0; i<program->nb_stream_indexes; i++) { - s->streams[program->stream_index[i]]->pts_wrap_reference = pts_wrap_reference; - s->streams[program->stream_index[i]]->pts_wrap_behavior = pts_wrap_behavior; + int64_t *stream_pts_wrap_reference = &(s->streams[program->stream_index[i]]->pts_wrap_reference); + int *stream_pts_wrap_behavior = &(s->streams[program->stream_index[i]]->pts_wrap_behavior); + + if (*stream_pts_wrap_reference != AV_NOPTS_VALUE && + (*stream_pts_wrap_reference - pts_wrap_reference > 1ULL << (st->pts_wrap_bits - 3) || + *stream_pts_wrap_reference < pts_wrap_reference)) { + /* + * If we find a defined wrap reference that is + * considerably larger, or that is smaller than the + * current default wrap reference, we update + * the default to current stream's wrap reference. + */ + av_log(s, AV_LOG_DEBUG, + "Updating default PTS wrap reference " + "%"PRId64" and PTS wrap behavior %d to " + "stream PTS wrap reference %"PRId64" and " + "PTS wrap behavior %d (program: %d, stream:%d, " + "stream_id: %d)\n", pts_wrap_reference, + pts_wrap_behavior, + *stream_pts_wrap_reference, + *stream_pts_wrap_behavior, + program->id, program->stream_index[i], + s->streams[program->stream_index[i]]->id); + + pts_wrap_reference = *stream_pts_wrap_reference; + pts_wrap_behavior = *stream_pts_wrap_behavior; + } else { + /* + * Otherwise, we just utilize and override the + * stream's wrap-around reference with the default + * wrap-around reference. + */ + *stream_pts_wrap_reference = pts_wrap_reference; + *stream_pts_wrap_behavior = pts_wrap_behavior; + } } program->pts_wrap_reference = pts_wrap_reference;
From: Masaki Tanaka <maki.rxrz@gmail.com> Seems to fix mistaken cases of discontinuity handling in MPEG-TS when program structure changes. Additional changes to patch from its original state by Jan Ekström. --- Had been meaning to post this for comments/discussion for a while, as this seems to make timestamps continuously rising in at least one of my samples where the program structure changes mid-stream. The original version of this patch can be found at: https://github.com/jeeb/ffmpeg/commit/6117366eaadbaf48bbd88eb2a353dfc852ff3400 The sample for which this helped is available at: https://kuroko.fushizen.eu/samples/pid_switch_sample.ts The difference of timestamps received from libavformat can be seen with: https://kuroko.fushizen.eu/screenshots/ffmpeg/blue_vanilla-orange_patched.png Jan libavformat/utils.c | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-)