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). - 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".
