| Message ID | 423ea570-a0a3-ece0-c2e7-d5ecf7d3e0b1@gyani.pro |
|---|---|
| State | New |
| Headers | show |
On Sat, Aug 24, 2019 at 11:28 AM Gyan <ffmpeg@gyani.pro> wrote: > > Fixes #8091, which reports what I consider a change rather than a > regression since 11d3b03fcb2baae4324aac9481b9bd4a171d4345 > <http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=11d3b03fcb2baae4324aac9481b9bd4a171d4345> > > It may be good practice to set default disposition in all demuxers which > only accept one stream of a type. > Any disposition should only be set if the container has any metadata to indicate them, not blindly be set for no particular reason. For the ticket in question, if you want to replace a stream in a file, you should manually ensure its mapped properly. - Hendrik
On 24-08-2019 04:43 PM, Hendrik Leppkes wrote: > On Sat, Aug 24, 2019 at 11:28 AM Gyan <ffmpeg@gyani.pro> wrote: >> Fixes #8091, which reports what I consider a change rather than a >> regression since 11d3b03fcb2baae4324aac9481b9bd4a171d4345 >> <http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=11d3b03fcb2baae4324aac9481b9bd4a171d4345> >> >> It may be good practice to set default disposition in all demuxers which >> only accept one stream of a type. >> > Any disposition should only be set if the container has any metadata > to indicate them, not blindly be set for no particular reason. > For the ticket in question, if you want to replace a stream in a file, > you should manually ensure its mapped properly. The ticket reports a change in auto-mapping behaviour. Manual mapping is a workaround which I already provided at trac but it doesn't correct the regression which should be addressed since I've confirmed that auto-mapping is affected for instances beyond that in the ticket e.g. ffmpeg -i avi -i mp4 outfile where both files have streams with identical parameters. Maybe a better change is to revert or refine 11d3b03fcb2 since disposition was originally introduced to convey and store flags in two particular formats: nut and matroska but the ffmpeg tool now unconditionally gives *default disposition* weight in selecting streams. Note: the MOV demuxer abuses the field by setting default disposition based on whether a stream is enabled so you can have a MP4 with N streams of a type, all being marked as default. For containers which only support one stream of a type (A, V or S) or raw bitstreams, metadata can't be expected since there's no choice or preference to be signaled; it is implicit that the one and only possible track is the 'default' one, so it is not a blind or wayward assignment. But this patch was a narrow fix for 8091 so I don't care to press for it. Gyan
Gyan (12019-08-24): > The ticket reports a change in auto-mapping behaviour. Manual mapping is a > workaround which I already provided at trac but it doesn't correct the > regression which should be addressed since I've confirmed that auto-mapping > is affected for instances beyond that in the ticket e.g. ffmpeg -i avi -i > mp4 outfile where both files have streams with identical parameters. Is it really a regression? I mean: was the old behaviour really better in many cases? Regards,
On 24-08-2019 07:07 PM, Nicolas George wrote: > Gyan (12019-08-24): >> The ticket reports a change in auto-mapping behaviour. Manual mapping is a >> workaround which I already provided at trac but it doesn't correct the >> regression which should be addressed since I've confirmed that auto-mapping >> is affected for instances beyond that in the ticket e.g. ffmpeg -i avi -i >> mp4 outfile where both files have streams with identical parameters. > Is it really a regression? I mean: was the old behaviour really better > in many cases? I see default disposition set by only three demuxers among the many dozens lavf has. Given the weight to it now, lowest index is no longer respected for otherwise near-identical streams. For audio it looks to be completely broken, e.g. a stereo stream can be chosen over an 8-channel stream. Gyan
diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c index 258f19174b..2199e42cd0 100644 --- a/libavformat/mp3dec.c +++ b/libavformat/mp3dec.c @@ -364,6 +364,7 @@ static int mp3_read_header(AVFormatContext *s) st->codecpar->codec_id = AV_CODEC_ID_MP3; st->need_parsing = AVSTREAM_PARSE_FULL_RAW; st->start_time = 0; + st->disposition |= AV_DISPOSITION_DEFAULT; // lcm of all mp3 sample rates avpriv_set_pts_info(st, 64, 1, 14112000);
Fixes #8091, which reports what I consider a change rather than a regression since 11d3b03fcb2baae4324aac9481b9bd4a171d4345 <http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=11d3b03fcb2baae4324aac9481b9bd4a171d4345> It may be good practice to set default disposition in all demuxers which only accept one stream of a type. Gyan From 647968016d28114bd9880daf5b5b728d005e8a69 Mon Sep 17 00:00:00 2001 From: Gyan Doshi <ffmpeg@gyani.pro> Date: Sat, 24 Aug 2019 14:52:31 +0530 Subject: [PATCH] avformat/mp3dec: set default disposition Fixes #8091 --- libavformat/mp3dec.c | 1 + 1 file changed, 1 insertion(+)