Paul B Mahol: > On Sun, Sep 12, 2021 at 12:02 AM Andreas Rheinhardt < > andreas.rheinha...@outlook.com> wrote: > >> Soft Works: >>> >>> >>>> -----Original Message----- >>>> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of >>>> Paul B Mahol >>>> Sent: Saturday, 11 September 2021 23:45 >>>> To: FFmpeg development discussions and patches <ffmpeg- >>>> de...@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 <softwo...@hotmail.com> >>>> wrote: >>>> >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of >>>>>> Hendrik Leppkes >>>>>> Sent: Saturday, 11 September 2021 23:42 >>>>>> To: FFmpeg development discussions and patches <ffmpeg- >>>>>> de...@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 >>>> <softwo...@hotmail.com> >>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On >>>> Behalf Of >>>>>>>> Paul B Mahol >>>>>>>> Sent: Saturday, 11 September 2021 12:51 >>>>>>>> To: FFmpeg development discussions and patches <ffmpeg- >>>>>>>> de...@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. >
Yes, and therefore the media type should be read from the AVFrame, not from an additional parameter to av_frame_get_buffer2. As I said. - Andreas _______________________________________________ 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".
