[FFmpeg-devel] [PATCH 2/2] avformat/movenc: support Opus packets with more than 60ms of audio when writing the Sample Group Description Box

Hendrik Leppkes h.leppkes at gmail.com
Fri Aug 24 18:57:33 EEST 2018


On Fri, Aug 24, 2018 at 5:53 PM James Almer <jamrial at gmail.com> wrote:
>
> On 8/24/2018 12:47 PM, Carl Eugen Hoyos wrote:
> > 2018-08-24 17:41 GMT+02:00, James Almer <jamrial at gmail.com>:
> >> On 8/24/2018 12:33 PM, Carl Eugen Hoyos wrote:
> >>> 2018-08-24 17:31 GMT+02:00, James Almer <jamrial at gmail.com>:
> >>>> On 8/24/2018 7:19 AM, Carl Eugen Hoyos wrote:
> >>>>> 2018-08-24 0:17 GMT+02:00, James Almer <jamrial at gmail.com>:
> >>>>>> Fixes assertion failures when trying to mux such streams.
> >>>>>
> >>>>> Shouldn't this be 1/2?
> >>>>>
> >>>>> And does this assert now for libavformat users that use
> >>>>> new libopus (but not libavcodec) or do I misunderstand?
> >>>>
> >>>> This asserts for any stream with >= 80ms packets. It doesn't need to be
> >>>> a direct encode from the libopus wrapper, since it can also happen
> >>>> during be a remux.
> >>>
> >>> Doesn't this indicate that the assert is wrong?
> >>> (That invalid input can trigger the assert)
> >>
> >> Invalid input (say, a packet reporting a frame size of the equivalent of
> >> 1ms) would assert before and after this patch. Do you consider an assert
> >> that triggers on invalid input to be wrong?
> >
> > I wanted to write "definitely", I am a little puzzled now that you seem to
> > disagree.
> >
> > Yes, I think so: It probably depends on the definition of "invalid" but
> > I mean invalid data with valid api usage.
> >
> > Carl Eugen
>
> I am completely lost in this discussion since i don't understand your
> concern about this at all, so please tell me what you want me to do so
> we can move on: Do i commit the patch as is, do i remove the assert line
> altogether, or do i replace it with a normal check?

asserts should guard against errors made by developers, invalid
internal API use, verifying assumptions made about how an API is being
used, and things like that.
File data should never be checked with an assert, because you can then
abort the process with a crafted file.

So I would suggest to replace it with a normal if check.

- Hendrik


More information about the ffmpeg-devel mailing list