[FFmpeg-devel] [PATCH v16 15/16] avcodec/subtitles: Migrate subtitle encoders to frame-based API and provide a compatibility shim for the legacy api

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Sat Nov 27 12:04:29 EET 2021


Soft Works:
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Andreas
>> Rheinhardt
>> Sent: Friday, November 26, 2021 4:09 PM
>> To: ffmpeg-devel at ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH v16 15/16] avcodec/subtitles: Migrate
>> subtitle encoders to frame-based API and provide a compatibility shim for the
>> legacy api
>>
>> You ported all subtitle encoders at once; this has the downside of
>> making the diff bigger and reviewing harder,
> 
> Nah, the changes are always the same, that's simple and straightforward.
> 

But they are in one big commit and that alone makes it harder. Of course
the combined diff is not bigger, but actually smaller (see below) when
porting everything in one commit.

>> but the advantage of
>> simplifying the compatibility code, because one knows that one does not
>> need to care about the case of an encoder implementing the old API. Yet
>> weirdly you didn't take advantage of this. You also did not remove
>> AVCodec.encode_sub.
> 
> Are you saying I do not need to keep the compatibility layer for "legacy"
> subtitle encoders?
> 
> I thought it's mandatory to keep it, but I'd be glad to remove..
> 

You need to keep the external API around for a while and this will force
some internal compatibility layer; but you also added unnecessary
internal compatibility layers: In both your new
avcodec_encode_subtitle2() as well as the old avcodec_encode_subtitle()
you do not need to care about what happens if the encoder implements the
old encode_sub API, because it no encoder implements said API any more.
You can (and should) remove AVCodec.encode_sub if you port all of its
users to encode2.

- Andreas


More information about the ffmpeg-devel mailing list