[FFmpeg-devel] [PATCH] lavf/matroskadec: stop mapping text/plain attachments to AV_CODEC_ID_TEXT

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Wednesday, June 8, 2022 8:17 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: stop mapping text/plain
> attachments to AV_CODEC_ID_TEXT
> 
> Quoting Soft Works (2022-06-08 05:45:38)
> > You might allow me the question whether we can be sure that
> > this is the only case which is subject to the regression?
> >
> > Besides from what I reported above (and you might probably come up
> > with a new codec ID for discriminating text subs vs. text
> > attachments), this surely fixes the specific case I reported,
> > but I wonder whether other cases could exist?
> > (it's meant to be a normal question - I just don't know)
> 
> We can never be truly sure of anything except our own existence.
> 
> As far as I can tell, only two bits of code in lavf can export
> AVMEDIA_TYPE_ATTACHMENT: apetag and matroskadec. apetag does not set
> codec id at all, while matroskadec will now only export codec ids that
> do not (and most likely will not) have decoders: fonts and generic
> binary data.
> 
> So after this patch, to the best of my knowledge, there should never be
> a case where an AVMEDIA_TYPE_ATTACHMENT stream has a decodable codec id.
> Then again this does not exclude all possible cases of a mismatch
> between a stream's codec type and id.

OK, that sounds fine.


> Overall I'd say this just strengthens the case for my original lavc
> commit, since it is clearly helpful in exposing bugs in other code.

As said already, I never doubted the validity of your patch, it was
about the effect and unacknowledged responsibilities.

What do you want to do with the text attachment "none" caption?
Maybe a separate "cummy" codec id?


> > Here's another thought that might be worth consideration:
> > What turned this from a minor into a major issue (from my pov),
> > is that it is causing ffprobe to fail and exit with error
> > and incomplete output.
> > What I'm wondering about in this context, is whether it
> > even has to be like that?
> >
> > I mean, an unknown codec doesn't cause ffprobe to error-exit,
> > does it need to do so when avcodec_open2() returns error?
> >
> > I would find that behavior ok and consistent when the same
> > would happen when running ffmpeg (ffprobe fails <=> ffmpeg fails).
> > But ffmpeg doesn't fail (unless you use the stream), so does
> > ffprobe even need to fail in these cases?
> 
> I suppose it can make sense to log an error and continue when opening
> the codec fails. This could be useful also for probing genuinely broken
> streams where e.g. extradata parsing fails.
> 
> There could also be an option like ffmpeg's -xerror that would make
> ffprobe exit on failure.

Sounds good to me, but I'm not sure whether everybody would be ok
doing it exactly like this, as somebody might argue they would rely
on ffprobe failing in such cases. 
I can submit a patch for that - unless no objections or better ideas
would appear..

Thanks,
sw