| Message ID | 20180119195145.6192-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | 55ebf707d0abf720a2b02fc5ebc47eb742bfbe99 |
| Headers | show |
On 19 January 2018 at 19:51, James Almer <jamrial@gmail.com> wrote: > Attempting full frame reconstruction is unnecessary for containers > like Matroska, so just skip it altogether. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/mlp_parser.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/libavcodec/mlp_parser.c b/libavcodec/mlp_parser.c > index 3c0330f777..4827354f18 100644 > --- a/libavcodec/mlp_parser.c > +++ b/libavcodec/mlp_parser.c > @@ -256,6 +256,9 @@ static int mlp_parse(AVCodecParserContext *s, > if (buf_size == 0) > return 0; > > + if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) { > + next = buf_size; > + } else { > if (!mp->in_sync) { > // Not in sync - find a major sync header > > @@ -315,6 +318,7 @@ static int mlp_parse(AVCodecParserContext *s, > } > > mp->bytes_left = 0; > + } > > sync_present = (AV_RB32(buf + 4) & 0xfffffffe) == 0xf8726fba; > > -- > 2.15.0 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Makes sense to me, both commits look fine
On Fri, 19 Jan 2018 16:51:45 -0300 James Almer <jamrial@gmail.com> wrote: > Attempting full frame reconstruction is unnecessary for containers > like Matroska, so just skip it altogether. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/mlp_parser.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/libavcodec/mlp_parser.c b/libavcodec/mlp_parser.c > index 3c0330f777..4827354f18 100644 > --- a/libavcodec/mlp_parser.c > +++ b/libavcodec/mlp_parser.c > @@ -256,6 +256,9 @@ static int mlp_parse(AVCodecParserContext *s, > if (buf_size == 0) > return 0; > > + if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) { > + next = buf_size; > + } else { > if (!mp->in_sync) { > // Not in sync - find a major sync header > > @@ -315,6 +318,7 @@ static int mlp_parse(AVCodecParserContext *s, > } > > mp->bytes_left = 0; > + } > > sync_present = (AV_RB32(buf + 4) & 0xfffffffe) == 0xf8726fba; > Not so sure about this. I think some mkv TrueHD files contain packets that are not properly starting on frame boundaries. But I don't really remember.
On 1/19/2018 5:06 PM, wm4 wrote: > On Fri, 19 Jan 2018 16:51:45 -0300 > James Almer <jamrial@gmail.com> wrote: > >> Attempting full frame reconstruction is unnecessary for containers >> like Matroska, so just skip it altogether. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/mlp_parser.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/libavcodec/mlp_parser.c b/libavcodec/mlp_parser.c >> index 3c0330f777..4827354f18 100644 >> --- a/libavcodec/mlp_parser.c >> +++ b/libavcodec/mlp_parser.c >> @@ -256,6 +256,9 @@ static int mlp_parse(AVCodecParserContext *s, >> if (buf_size == 0) >> return 0; >> >> + if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) { >> + next = buf_size; >> + } else { >> if (!mp->in_sync) { >> // Not in sync - find a major sync header >> >> @@ -315,6 +318,7 @@ static int mlp_parse(AVCodecParserContext *s, >> } >> >> mp->bytes_left = 0; >> + } >> >> sync_present = (AV_RB32(buf + 4) & 0xfffffffe) == 0xf8726fba; >> > > Not so sure about this. I think some mkv TrueHD files contain packets > that are not properly starting on frame boundaries. But I don't really > remember. As in badly muxed files? Couldn't the same thing happen with any other codec? We're not bothering to consider such cases in other parses as far as i could see.
On Fri, Jan 19, 2018 at 08:30:59PM -0300, James Almer wrote: > On 1/19/2018 5:06 PM, wm4 wrote: > > On Fri, 19 Jan 2018 16:51:45 -0300 > > James Almer <jamrial@gmail.com> wrote: > > > >> Attempting full frame reconstruction is unnecessary for containers > >> like Matroska, so just skip it altogether. > >> > >> Signed-off-by: James Almer <jamrial@gmail.com> > >> --- > >> libavcodec/mlp_parser.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/libavcodec/mlp_parser.c b/libavcodec/mlp_parser.c > >> index 3c0330f777..4827354f18 100644 > >> --- a/libavcodec/mlp_parser.c > >> +++ b/libavcodec/mlp_parser.c > >> @@ -256,6 +256,9 @@ static int mlp_parse(AVCodecParserContext *s, > >> if (buf_size == 0) > >> return 0; > >> > >> + if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) { > >> + next = buf_size; > >> + } else { > >> if (!mp->in_sync) { > >> // Not in sync - find a major sync header > >> > >> @@ -315,6 +318,7 @@ static int mlp_parse(AVCodecParserContext *s, > >> } > >> > >> mp->bytes_left = 0; > >> + } > >> > >> sync_present = (AV_RB32(buf + 4) & 0xfffffffe) == 0xf8726fba; > >> > > > > Not so sure about this. I think some mkv TrueHD files contain packets > > that are not properly starting on frame boundaries. But I don't really > > remember. > > As in badly muxed files? Couldn't the same thing happen with any other > codec? We're not bothering to consider such cases in other parses as far > as i could see. I agree, also IIRC we generally put workarounds for this kind of thing in demuxers. there both demmuxer and codec is known [...]
On Fri, 19 Jan 2018 20:30:59 -0300 James Almer <jamrial@gmail.com> wrote: > On 1/19/2018 5:06 PM, wm4 wrote: > > On Fri, 19 Jan 2018 16:51:45 -0300 > > James Almer <jamrial@gmail.com> wrote: > > > >> Attempting full frame reconstruction is unnecessary for containers > >> like Matroska, so just skip it altogether. > >> > >> Signed-off-by: James Almer <jamrial@gmail.com> > >> --- > >> libavcodec/mlp_parser.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/libavcodec/mlp_parser.c b/libavcodec/mlp_parser.c > >> index 3c0330f777..4827354f18 100644 > >> --- a/libavcodec/mlp_parser.c > >> +++ b/libavcodec/mlp_parser.c > >> @@ -256,6 +256,9 @@ static int mlp_parse(AVCodecParserContext *s, > >> if (buf_size == 0) > >> return 0; > >> > >> + if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) { > >> + next = buf_size; > >> + } else { > >> if (!mp->in_sync) { > >> // Not in sync - find a major sync header > >> > >> @@ -315,6 +318,7 @@ static int mlp_parse(AVCodecParserContext *s, > >> } > >> > >> mp->bytes_left = 0; > >> + } > >> > >> sync_present = (AV_RB32(buf + 4) & 0xfffffffe) == 0xf8726fba; > >> > > > > Not so sure about this. I think some mkv TrueHD files contain packets > > that are not properly starting on frame boundaries. But I don't really > > remember. > > As in badly muxed files? Couldn't the same thing happen with any other > codec? We're not bothering to consider such cases in other parses as far > as i could see. From my hazy memory, this also happened with mp3, although the Matroska demuxer explicitly forces full parsing for that codec.
On 1/20/2018 7:15 AM, wm4 wrote: > On Fri, 19 Jan 2018 20:30:59 -0300 > James Almer <jamrial@gmail.com> wrote: > >> On 1/19/2018 5:06 PM, wm4 wrote: >>> On Fri, 19 Jan 2018 16:51:45 -0300 >>> James Almer <jamrial@gmail.com> wrote: >>> >>>> Attempting full frame reconstruction is unnecessary for containers >>>> like Matroska, so just skip it altogether. >>>> >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> libavcodec/mlp_parser.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/libavcodec/mlp_parser.c b/libavcodec/mlp_parser.c >>>> index 3c0330f777..4827354f18 100644 >>>> --- a/libavcodec/mlp_parser.c >>>> +++ b/libavcodec/mlp_parser.c >>>> @@ -256,6 +256,9 @@ static int mlp_parse(AVCodecParserContext *s, >>>> if (buf_size == 0) >>>> return 0; >>>> >>>> + if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) { >>>> + next = buf_size; >>>> + } else { >>>> if (!mp->in_sync) { >>>> // Not in sync - find a major sync header >>>> >>>> @@ -315,6 +318,7 @@ static int mlp_parse(AVCodecParserContext *s, >>>> } >>>> >>>> mp->bytes_left = 0; >>>> + } >>>> >>>> sync_present = (AV_RB32(buf + 4) & 0xfffffffe) == 0xf8726fba; >>>> >>> >>> Not so sure about this. I think some mkv TrueHD files contain packets >>> that are not properly starting on frame boundaries. But I don't really >>> remember. >> >> As in badly muxed files? Couldn't the same thing happen with any other >> codec? We're not bothering to consider such cases in other parses as far >> as i could see. > > From my hazy memory, this also happened with mp3, although the Matroska > demuxer explicitly forces full parsing for that codec. I guess i could force full parsing for truehd on matroska as well, but it kinda defeats the point of this patch. There are not a lot of containers that support it after all (mpegts is another, but that one forces full parsing for everything). Would that be ok?
diff --git a/libavcodec/mlp_parser.c b/libavcodec/mlp_parser.c index 3c0330f777..4827354f18 100644 --- a/libavcodec/mlp_parser.c +++ b/libavcodec/mlp_parser.c @@ -256,6 +256,9 @@ static int mlp_parse(AVCodecParserContext *s, if (buf_size == 0) return 0; + if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) { + next = buf_size; + } else { if (!mp->in_sync) { // Not in sync - find a major sync header @@ -315,6 +318,7 @@ static int mlp_parse(AVCodecParserContext *s, } mp->bytes_left = 0; + } sync_present = (AV_RB32(buf + 4) & 0xfffffffe) == 0xf8726fba;
Attempting full frame reconstruction is unnecessary for containers like Matroska, so just skip it altogether. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/mlp_parser.c | 4 ++++ 1 file changed, 4 insertions(+)