[FFmpeg-devel] [PATCH 4/6] lavc: support multiple frames in a singe AVPacket in avcodec_decode_subtitle2
Marton Balint
cus at passwd.hu
Wed Oct 16 02:09:34 CEST 2013
On Tue, 15 Oct 2013, Clément Bœsch wrote:
> On Sat, Oct 05, 2013 at 07:34:56PM +0200, Marton Balint wrote:
>> This feature is also known as CODEC_CAP_SUBFRAMES support. The patch also adds
>> CODEC_CAP_DELAY support to avcodec_decode_subtitle2.
>>
>> For DVB teletext decoding, a single teletext packet can contain multiple
>> teletext pages. In order to support that, we extend the API the same way it is
>> used now for audio decoding.
>>
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>> doc/APIchanges | 4 ++++
>> ffmpeg.c | 2 +-
>> libavcodec/avcodec.h | 13 +++++++++++++
>> libavcodec/utils.c | 11 +++++++++--
>> libavcodec/version.h | 2 +-
>> libavformat/utils.c | 1 -
>> 6 files changed, 28 insertions(+), 5 deletions(-)
>>
>
> I think that changes the behaviour too much, I would suggest to introduce
> avcodec_decode_subtitle3()
>
Okay, I can certainly do that. However I am not sure what
avcodec_decode_subtitle2 should do with codecs with CODEC_CAP_DELAY or
CODEC_CAP_SUBFRAMES?
Return AVERROR(ENOSYS) for all calls? Or only return AVERROR(ENOSYS) if
multiple frames can be decoded from the packet? Or always return the
first frame and skip the rest, and report no error? What do you think?
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index dc43313..1706c97 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -15,6 +15,10 @@ libavutil: 2012-10-22
>>
>> API changes, most recent first:
>>
>> +2013-10-xx - xxxxxxx - lavc 55.35.100 - avcodec.h
>> + Add CODEC_CAP_DELAY and CODEC_CAP_SUBFRAMES support to
>> + avcodec_decode_subtitle2.
>> +
>> 2013-10-03 - xxxxxxx - lavc 55.34.100 - avcodec.h
>> Add av_codec_get_max_lowres()
>>
>> diff --git a/ffmpeg.c b/ffmpeg.c
>> index d1c841f..47c94aa 100644
>> --- a/ffmpeg.c
>> +++ b/ffmpeg.c
>> @@ -1897,7 +1897,7 @@ static int output_packet(InputStream *ist, const AVPacket *pkt)
>>
>> // touch data and size only if not EOF
>> if (pkt) {
>> - if(ist->st->codec->codec_type != AVMEDIA_TYPE_AUDIO)
>> + if(ist->st->codec->codec_type == AVMEDIA_TYPE_VIDEO)
>> ret = avpkt.size;
>> avpkt.data += ret;
>> avpkt.size -= ret;
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index 2917a2f..5447a8e 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -3875,11 +3875,24 @@ int avcodec_decode_video2(AVCodecContext *avctx, AVFrame *picture,
>> * Return a negative value on error, otherwise return the number of bytes used.
>> * If no subtitle could be decompressed, got_sub_ptr is zero.
>> * Otherwise, the subtitle is stored in *sub.
>> + *
>> + * Some decoders may support multiple frames in a single AVPacket. Such
>> + * decoders would then just decode the first frame. In this case,
>> + * avcodec_decode_subtitle2 has to be called again with an AVPacket containing
>> + * the remaining data in order to decode the second frame, etc...
>> + * Even if no frames are returned, or a frame is returned but no data is
>> + * consumed, the packet needs to be fed to the decoder with remaining data
>> + * until it is completely consumed or an error occurs.
>> + *
>> * Note that CODEC_CAP_DR1 is not available for subtitle codecs. This is for
>> * simplicity, because the performance difference is expect to be negligible
>> * and reusing a get_buffer written for video codecs would probably perform badly
>> * due to a potentially very different allocation pattern.
>> *
>> + * @note Codecs which have the CODEC_CAP_DELAY capability set have a delay
>> + * between input and output, these need to be fed with avpkt->data=NULL,
>> + * avpkt->size=0 at the end to return the remaining frames.
>> + *
>
> This looks like to be from the doxy of the deprecated audio decoding (v3);
> why not update the doxy using the v4? Is there a behaviour difference?
>
I can change that, I think the docs in v3 and v4 pretty much say the same
thing in this regard, only wording is different a bit. There is one place
where I extended the docs, it is where I explicitly declared that
consuming no bytes from a packet and returning a frame at the same time is
a valid combination.
By the way, to fight infinite loops in decoders shouldn't we state that
consuming 0 bytes and returning no frames should never happen for ordinary
packets and we should check for that in avcodec_decode_subtitle3 and
return an error if it does?
>> * @param avctx the codec context
>> * @param[out] sub The AVSubtitle in which the decoded subtitle will be stored, must be
>> freed with avsubtitle_free if *got_sub_ptr is set.
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index 20de48e..ea30c39 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -2330,15 +2330,22 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
>> {
>> int i, ret = 0;
>>
>> + *got_sub_ptr = 0;
>> +
>> + if (!avpkt->data && avpkt->size) {
>> + av_log(avctx, AV_LOG_ERROR, "invalid packet: NULL data, size != 0\n");
>> + return AVERROR(EINVAL);
>> + }
>
>> + if (!avctx->codec)
>> + return AVERROR(EINVAL);
>
> Can that happen? Why that sudden check?
>
Copy pasted from avcodec_decode_audio4. On second look, it seems unrelated
to the CODEC_CAP_DELAY stuff, I can remove it from this patch. For audio,
it is there because of ticket 2722. (under memory pressure, it can happen)
>> if (avctx->codec->type != AVMEDIA_TYPE_SUBTITLE) {
>> av_log(avctx, AV_LOG_ERROR, "Invalid media type for subtitles\n");
>> return AVERROR(EINVAL);
>> }
>>
>> - *got_sub_ptr = 0;
>> avcodec_get_subtitle_defaults(sub);
>>
>> - if (avpkt->size) {
>> + if ((avctx->codec->capabilities & CODEC_CAP_DELAY) || avpkt->size) {
>> AVPacket pkt_recoded;
>> AVPacket tmp = *avpkt;
>> int did_split = av_packet_split_side_data(&tmp);
>> diff --git a/libavcodec/version.h b/libavcodec/version.h
>> index 7f9682e..63a2d8f 100644
>> --- a/libavcodec/version.h
>> +++ b/libavcodec/version.h
>> @@ -29,7 +29,7 @@
>> #include "libavutil/avutil.h"
>>
>> #define LIBAVCODEC_VERSION_MAJOR 55
>> -#define LIBAVCODEC_VERSION_MINOR 34
>> +#define LIBAVCODEC_VERSION_MINOR 35
>> #define LIBAVCODEC_VERSION_MICRO 100
>>
>> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> index 61405d7..becac14 100644
>> --- a/libavformat/utils.c
>> +++ b/libavformat/utils.c
>> @@ -2512,7 +2512,6 @@ static int try_decode_frame(AVFormatContext *s, AVStream *st, AVPacket *avpkt, A
>> case AVMEDIA_TYPE_SUBTITLE:
>> ret = avcodec_decode_subtitle2(st->codec, &subtitle,
>> &got_picture, &pkt);
>> - ret = pkt.size;
>> break;
>> default:
>> break;
>> --
>> 1.8.1.4
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> --
> Clément B.
>
Regards,
Marton
More information about the ffmpeg-devel
mailing list