> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
> Soft Works
> Sent: Wednesday, October 6, 2021 2:29 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v3 1/2] avcodec/codec_par: Add
> codec properties field to AVCodecParameters
>
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
> > Andreas Rheinhardt
> > Sent: Wednesday, October 6, 2021 12:18 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v3 1/2] avcodec/codec_par: Add
> > codec properties field to AVCodecParameters
> >
> > Soft Works:
> > >
> > >
> > >> -----Original Message-----
> > >> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf
> Of
> > >> Andreas Rheinhardt
> > >> Sent: Wednesday, October 6, 2021 11:30 AM
> > >> To: ffmpeg-devel@ffmpeg.org
> > >> Subject: Re: [FFmpeg-devel] [PATCH v3 1/2] avcodec/codec_par:
> Add
> > >> codec properties field to AVCodecParameters
> > >>
> > >> Soft Works:
> > >>>
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf
> > Of
> > >>>> Hendrik Leppkes
> > >>>> Sent: Wednesday, October 6, 2021 8:57 AM
> > >>>> To: FFmpeg development discussions and patches <ffmpeg-
> > >>>> de...@ffmpeg.org>
> > >>>> Subject: Re: [FFmpeg-devel] [PATCH v3 1/2] avcodec/codec_par:
> > Add
> > >>>> codec properties field to AVCodecParameters
> > >>>>
> > >>>> On Wed, Oct 6, 2021 at 8:45 AM Soft Works
> > <softwo...@hotmail.com>
> > >>>> wrote:
> > >>>>> diff --git a/libavcodec/codec_par.h b/libavcodec/codec_par.h
> > >>>>> index 10cf79dff1..42ed8deb13 100644
> > >>>>> --- a/libavcodec/codec_par.h
> > >>>>> +++ b/libavcodec/codec_par.h
> > >>>>> @@ -198,6 +198,10 @@ typedef struct AVCodecParameters {
> > >>>>> * Audio only. Number of samples to skip after a
> > >>>> discontinuity.
> > >>>>> */
> > >>>>> int seek_preroll;
> > >>>>> + /**
> > >>>>> + * Codec properties of the stream that gets decoded
> > >>>>> + */
> > >>>>> + unsigned properties;
> > >>>>> } AVCodecParameters;
> > >>>>>
> > >>>>
> > >>>> This field is severly underspecified/underdocumented. I
> realize
> > >> you
> > >>>> just copied it, but if I'm looking at this without pre-
> existing
> > >>>> knowledge, then I have absolutely no idea what it might be
> for,
> > or
> > >>>> what kind of values go in there. The old field had the defines
> > of
> > >> the
> > >>>> possible bits right there for context - this doesn't have
> this,
> > so
> > >> it
> > >>>> would definitely benefit from added documentation.
> > >>>
> > >>> Hello Hendrik,
> > >>>
> > >>> that's right, but what would you suggest? We can't duplicate
> the
> > >> defines,
> > >>> though I could add a line in the help text like
> > >>>
> > >>> /**
> > >>> * Codec properties of the stream that gets decoded.
> > >>> * Corresponds to @ref AVCodecContext.properties
> > "properties".
> > >>> */
> > >>> unsigned properties;
> > >>>
> > >>> Would that be OK?
> > >>>
> > >>
> > >> The defines could be moved to defs.h (while just at it, we could
> > also
> > >> add properly prefixed alternatives and deprecate the FF_ ones).
> > >> The copying that you are adding in
> > avcodec_parameters_from_context
> > >> is
> > >> problematic, as the relevant AVCodecContext field is marked as
> > "set
> > >> by
> > >> libavcodec" for decoding, which means that decoders expect it to
> > be
> > >> blank initially.
> > >
> > > Why do you think that decoders would expect it to be blank
> > initially?
> > >
> > > What about the other fields then, like format, profile, level,
> > width,
> > > height, etc. - these are copied in the same way.
> > > Are you saying that decoders expect all other fields to be filled
> > but
> > > the properties field to be zero?
> > >
> >
> > Of course not. E.g. the documentation of the dimension explicitly
> > says
> > so: "May be set by the user before opening the decoder if known
> e.g.
> > from the container. Some decoders will require the dimensions to be
> > set
> > by the caller. During decoding, the decoder may overwrite those
> > values
> > as required while parsing the data."
> > The semantics of profile and level are closer, yet the difference
> is
> > that if the decoder sets these fields, it overwrites any garbage it
> > may
> > have had; this is not so with a bitfield like properties.
> >
> > > Also, the docs say that it's SET by decoders, not READ by
> decoders,
> > > which means that no decoder could rely on this field. There's
> also
> > > no undefined-state defined (like -1), which means that a decoder
> > > couldn't know whether a 0-value indicates a valid value or a not-
> > yet
> > > initialized value.
> > >
> >
> > You completely misunderstood what my concern was about: In the
> > scenario
> > described above an API user querying the properties field would get
> > the
> > information that this stream contains CC data; even when it
> doesn't.
> >
> > And your other claims are also wrong: A decoder may read anything
> > from
> > an AVCodecContext or from the AVPackets and AVFrames with the
> > exception
> > of data that is opaque to it. And if a field is described as
> > out-of-touch by the user, then the codec may rely on it not having
> > arbitrary values, but only the values it has set itself (or the
> > default
> > value). Notice that if the user initializes the AVCodecContext via
> > avcodec_parameters_from_context(), then it still counts as user-set
> > even
> > though said function lives in libavcodec, too.
>
> When you think that this is the wrong way, what would you suggest?
Hi Andreas,
even though I do not share the concern you mentioned, one way to address
this would be to include the properties field only when copying from
codec context to codec_par but not in the other direction.
Would that be better from your point of view?
Thanks,
softworkz
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
