[FFmpeg-devel] [PATCH v3 01/18] avutil/frame: Subtitle Filtering - Add AVMediaType property to AVFrame

Paul B Mahol onemda at gmail.com
Sun Sep 12 01:09:57 EEST 2021


On Sun, Sep 12, 2021 at 12:02 AM Andreas Rheinhardt <
andreas.rheinhardt at outlook.com> wrote:

> Soft Works:
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> >> Paul B Mahol
> >> Sent: Saturday, 11 September 2021 23:45
> >> To: FFmpeg development discussions and patches <ffmpeg-
> >> devel at ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH v3 01/18] avutil/frame: Subtitle
> >> Filtering - Add AVMediaType property to AVFrame
> >>
> >> On Sat, Sep 11, 2021 at 11:43 PM Soft Works <softworkz at hotmail.com>
> >> wrote:
> >>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> >>>> Hendrik Leppkes
> >>>> Sent: Saturday, 11 September 2021 23:42
> >>>> To: FFmpeg development discussions and patches <ffmpeg-
> >>>> devel at ffmpeg.org>
> >>>> Subject: Re: [FFmpeg-devel] [PATCH v3 01/18] avutil/frame:
> >> Subtitle
> >>>> Filtering - Add AVMediaType property to AVFrame
> >>>>
> >>>> On Sat, Sep 11, 2021 at 8:31 PM Soft Works
> >> <softworkz at hotmail.com>
> >>>> wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On
> >> Behalf Of
> >>>>>> Paul B Mahol
> >>>>>> Sent: Saturday, 11 September 2021 12:51
> >>>>>> To: FFmpeg development discussions and patches <ffmpeg-
> >>>>>> devel at ffmpeg.org>
> >>>>>> Subject: Re: [FFmpeg-devel] [PATCH v3 01/18] avutil/frame:
> >>>> Subtitle
> >>>>>> Filtering - Add AVMediaType property to AVFrame
> >>>>>>
> >>>>>>> @@ -721,6 +728,8 @@ void av_frame_move_ref(AVFrame *dst,
> >>>> AVFrame
> >>>>>> *src);
> >>>>>>>  /**
> >>>>>>>   * Allocate new buffer(s) for audio or video data.
> >>>>>>>   *
> >>>>>>> + * Note: For subtitle data, use av_frame_get_buffer2
> >>>>>>>
> >>>>>>
> >>>>>> I dislike this approach. It should be consistent with audio &
> >>>> video.
> >>>>>
> >>>>> I agree, it's not nice. I will just change av_frame_get_buffer
> >>>> instead.
> >>>>>
> >>>>
> >>>> We don't change existing functions like that. We add new API, or
> >>>> deprecate old ones, we don't change it in such an incompatible
> >>>> manner.
> >>>> But before these functions are even needed, lets just see if its
> >> even
> >>>> needed to change anything, depending on how subtitles will
> >> ultimately
> >>>> end up packaged.
> >>>
> >>> When you say, "We add new API", then my initial approach with
> >>> av_frame_get_buffer2 was correct?
> >>>
> >>> No. That is bad design.
> >>
> >> You do not need another argument for function.
> >>
> >> Use same function, same number of arguments and type.
> >> Just change internal stuff.
> >
> > The problem is that the current implementation is inferring the media
> > type by checking width and height (=> video, otherwise audio).
> >
> > Subtitle frames can have w+h or not, so it can't continue to work
> > this way.
> >
> > The idea of av_frame_get_buffer2 is to have an explicit AVMediaType
> > parameter to specify.
> >
> > An alternative would be to require the (new) type property to be
> > set explicitly in case of subtitles and continue the current way
> > of inference when it's not set.
> > That would work but it wouldn't be a clean API design either.
> >
> > What would you suggest?
> >
>
> Add an av_frame_get_buffer2 that requires the media type of the frame to
> be set (on the frame, not as an additional parameter).
>

why? AVFrame is supposed to have media type in itself.

>
> - Andreas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>


More information about the ffmpeg-devel mailing list