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

[Soft Works <softworkz@hotmail.com>]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Wednesday, June 8, 2022 9:09 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel@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 08:39:21)
> > > 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?
> 
> Do we need to do anything about it? I am not a fan of inventing fake
> codec ids for every conceivable kind of data. lavf already exports the
> MIME type, that should be enough. Maybe the way it is printed can be
> improved, but that is not urgent as far as I'm concerned.

ffprobe provides multiple output formats which are intended to be machine-
readable (e.g. xml, json).
If it was just about interactive reading by a user, then it might 
in fact not matter that much. But for a machine, it was "text" and the patch
changes it to "none". 

> I am not a fan of inventing fake
> codec ids for every conceivable kind of data.

This surely makes sense from a larger perspective, but when such fake ids are 
already being used, then it doesn't matter how many. 5 or 6 - 33 or 34 - that 
doesn't have relevant impact. At one point in the future, we might get rid of
them for something better, but - again - the count won't be of much relevance. 

Whereas implementing some special sauce, different from the other attachment
type handling just to avoid one more fake id - well, I'm not sure whether 
there would be any benefit in doing so.

But when you want to do some kind of workaround instead, I don't mind either,
I mean it's just about printing "text" instead of none.



Ah, when you remove the mapping at the decoding side, you should probably 
remove it at the other side as well:

https://github.com/FFmpeg/FFmpeg/blob/93505a9095bbacba9b2102a05191e0bc8ba36d4b/libavformat/matroskaenc.c#L2163-L2164


> > > 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..
> 
> https://xkcd.com/1172/

I don't think it's absurd, neither hypothetical to ask whether somebody,
would object a certain change --- BEFORE working spending time on it.


Thanks,
sw



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".