On Thu, Nov 03, 2016 at 01:04:21AM +0100, Andreas Cadhalpun wrote:
> On 03.11.2016 00:42, Michael Niedermayer wrote:
> > On Wed, Nov 02, 2016 at 10:30:30PM +0100, Andreas Cadhalpun wrote:
> >> On 02.11.2016 13:07, Michael Niedermayer wrote:
> >>> On Sat, Oct 22, 2016 at 01:16:00AM +0200, Andreas Cadhalpun wrote:
> >>>> utils.c | 12 ++++++++++++
> >>>> 1 file changed, 12 insertions(+)
> >>>> ffefc22756b774cb7652587207ae66cfbf681be3
> >>>> 0001-avformat-close-parser-if-codec-changed.patch
> >>>> From 9de87a4fb2c6c6311a11a2da5de8554a71adfa66 Mon Sep 17 00:00:00 2001
> >>>> From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
> >>>> Date: Mon, 17 Oct 2016 20:26:51 +0200
> >>>> Subject: [PATCH] avformat: close parser if codec changed
> >>>>
> >>>> The parser depends on the codec and thus must not be used with a
> >>>> different one.
> >>>> If it is, the 'avctx->codec_id == s->parser->codec_ids[0] ...' assert in
> >>>> av_parser_parse2 gets triggered.
> >>>>
> >>>> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
> >>>> ---
> >>>> libavformat/utils.c | 12 ++++++++++++
> >>>> 1 file changed, 12 insertions(+)
> >>>>
> >>>> diff --git a/libavformat/utils.c b/libavformat/utils.c
> >>>> index 70dbfa1..a8a78ed 100644
> >>>> --- a/libavformat/utils.c
> >>>> +++ b/libavformat/utils.c
> >>>> @@ -480,6 +480,12 @@ static int update_stream_avctx(AVFormatContext *s)
> >>>> if (!st->internal->need_context_update)
> >>>> continue;
> >>>>
> >>>> + /* close parser, because it depends on the codec */
> >>>> + if (st->parser && st->internal->avctx->codec_id !=
> >>>> st->codecpar->codec_id) {
> >>>> + av_parser_close(st->parser);
> >>>> + st->parser = NULL;
> >>>> + }
> >>>> +
> >>>> /* update internal codec context, for the parser */
> >>>> ret = avcodec_parameters_to_context(st->internal->avctx,
> >>>> st->codecpar);
> >>>> if (ret < 0)
> >>>> @@ -1515,6 +1521,12 @@ static int read_frame_internal(AVFormatContext
> >>>> *s, AVPacket *pkt)
> >>>> st->info->found_decoder = 0;
> >>>> }
> >>>>
> >>>> + /* close parser, because it depends on the codec */
> >>>> + if (st->parser && st->internal->avctx->codec_id !=
> >>>> st->codecpar->codec_id) {
> >>>> + av_parser_close(st->parser);
> >>>> + st->parser = NULL;
> >>>> + }
> >>>> +
> >>>
> >>> what if the codec id differs but both are supported by the parser ?
> >>> AVCodecParser has a list of 5 codec ids ?
> >>>
> >>> I didnt find a testcase where this makes a difference, just wondering
> >>
> >> I think the parser should be re-initialized in that case, too.
> >
> > doesnt this lead to data loss (parser internal buffers being lost at
> > a transition between 2 types)?
>
> Yes, but it's not clear that the parser internal state is still correct
> after a change of the codec id.what exact case are we talking about ? A. The parser changeing the codec_id ? B. The caller changing the codec_id ? B looks like a violation of the API, the caller cant change things without reopening the parser for A id consider the parser completely broken if it changes the codec id and falls in a inconsistent state without the user detecting that and closing it. Such requirement for user apps also is not documented Parsers dont handle unrelated formats, they handle things that are similar or identical like AV_CODEC_ID_MJPEG, AV_CODEC_ID_JPEGLS some parsers dont set the codec_id, like *jpeg doesnt mpeg audio contains checks to explicitly check for changing codec id only ac3 and mpeg2 seem to set the codec id at all which parser becomes inconsistent with itself when changing codec id ? > > > both types might be handled by the same decoder so nothing special > > might be needed "downstream" > > It might work for some decoders and might not work for others. > It's hard to tell without samples. making samples should be as easy as concatenating 2 streams if someone wants to check what happens exactly [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When the tyrant has disposed of foreign enemies by conquest or treaty, and there is nothing more to fear from them, then he is always stirring up some war or other, in order that the people may require a leader. -- Plato
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
