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