[FFmpeg-devel] [PATCH 1/2] allow passing subtitles header between decoder and encoder
Aurelien Jacobs
aurel
Sun Nov 7 23:44:03 CET 2010
On Mon, Oct 25, 2010 at 03:24:18AM +0200, Michael Niedermayer wrote:
> On Mon, Oct 25, 2010 at 12:04:59AM +0200, Aurelien Jacobs wrote:
> > On Sat, Oct 23, 2010 at 09:57:26PM +0200, Michael Niedermayer wrote:
> > > On Tue, Oct 19, 2010 at 09:55:47PM +0200, Aurelien Jacobs wrote:
> > > > On Tue, Oct 19, 2010 at 03:43:52PM +0200, Michael Niedermayer wrote:
> > > > > On Sat, Oct 16, 2010 at 05:31:26PM +0200, Aurelien Jacobs wrote:
> > > > > > ---
> > > > > > ffmpeg.c | 11 +++++++++++
> > > > > > libavcodec/avcodec.h | 8 ++++++++
> > > > > > libavcodec/utils.c | 2 ++
> > > > > > libavformat/utils.c | 4 ++++
> > > > > > 4 files changed, 25 insertions(+), 0 deletions(-)
> > > > > >
> > > > > > diff --git a/ffmpeg.c b/ffmpeg.c
> > > > > > index 4f59b2e..7622ccc 100644
> > > > > > --- a/ffmpeg.c
> > > > > > +++ b/ffmpeg.c
> > > > > > @@ -2365,6 +2365,7 @@ static int transcode(AVFormatContext **output_files,
> > > > > > ost = ost_table[i];
> > > > > > if (ost->encoding_needed) {
> > > > > > AVCodec *codec = i < nb_output_codecs ? output_codecs[i] : NULL;
> > > > > > + AVCodecContext *dec = ist_table[ost->source_index]->st->codec;
> > > > > > if (!codec)
> > > > > > codec = avcodec_find_encoder(ost->st->codec->codec_id);
> > > > > > if (!codec) {
> > > > > > @@ -2373,6 +2374,15 @@ static int transcode(AVFormatContext **output_files,
> > > > > > ret = AVERROR(EINVAL);
> > > > > > goto dump_format;
> > > > > > }
> > > > > > + if (dec->subtitle_header) {
> > > > > > + ost->st->codec->subtitle_header = av_malloc(dec->subtitle_header_size);
> > > > > > + if (!ost->st->codec->subtitle_header) {
> > > > > > + ret = AVERROR(ENOMEM);
> > > > > > + goto dump_format;
> > > > > > + }
> > > > > > + memcpy(ost->st->codec->subtitle_header, dec->subtitle_header, dec->subtitle_header_size);
> > > > > > + ost->st->codec->subtitle_header_size = dec->subtitle_header_size;
> > > > > > + }
> > > > > > if (avcodec_open(ost->st->codec, codec) < 0) {
> > > > > > snprintf(error, sizeof(error), "Error while opening encoder for output stream #%d.%d - maybe incorrect parameters such as bit_rate, rate, width or height",
> > > > > > ost->file_index, ost->index);
> > > > > > @@ -2728,6 +2738,7 @@ static int transcode(AVFormatContext **output_files,
> > > > > > }
> > > > > > av_fifo_free(ost->fifo); /* works even if fifo is not
> > > > > > initialized but set to zero */
> > > > > > + av_freep(&ost->st->codec->subtitle_header);
> > > > > > av_free(ost->pict_tmp.data[0]);
> > > > > > if (ost->video_resample)
> > > > > > sws_freeContext(ost->img_resample_ctx);
> > > > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > > > index 4bddbaa..ab3cbd9 100644
> > > > > > --- a/libavcodec/avcodec.h
> > > > > > +++ b/libavcodec/avcodec.h
> > > > > > @@ -2744,6 +2744,14 @@ typedef struct AVCodecContext {
> > > > > > * - decoding: unused
> > > > > > */
> > > > > > int lpc_passes;
> > > > > > +
> > > > > > + /**
> > > > > > + * Header containing style information for text subtitles.
> > > > > > + * - encoding: Set/allocated/freed by user (before avcodec_open())
> > > > > > + * - decoding: Set/allocated/freed by libavcodec (by avcodec_open())
> > > > > > + */
> > > > > > + uint8_t *subtitle_header;
> > > > > > + int subtitle_header_size;
> > > > > > } AVCodecContext;
> > > > > >
> > > > > > /**
> > > > >
> > > > > The description should be enough to implement a decoder and a user app and
> > > > > a subtitle renderer based on it. (some common sense and guessing not excluded)
> > > > > but this seems too terse for that
> > > >
> > > > OK. Updated patch.
> > > > I think it should now be enough to implement anything related to this
> > > > field, with a little bit of common sense and the ASS spec around.
> > > >
> > > > > also are you sure every subtitle decoder will be able to set this by
> > > > > avcodec_open() ?
> > > >
> > > > Yes, sure. It wouldn't be very useful anyway to set it later on.
> > > > I've already (started to) implement several decoders (SubRip,
> > > > MicroDVD, Movtext...), and they fits well within this model.
> > > >
> > > > Aurel
> > > >
> > > > [...]
> > > >
> > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > index 4bddbaa..99bc7ad 100644
> > > > --- a/libavcodec/avcodec.h
> > > > +++ b/libavcodec/avcodec.h
> > > > @@ -2744,6 +2744,17 @@ typedef struct AVCodecContext {
> > > > * - decoding: unused
> > > > */
> > > > int lpc_passes;
> > > > +
> > > > + /**
> > > > + * Header containing style information for text subtitles.
> > > > + * For SUBTITLE_ASS subtitle type, it should contain the whole ASS
> > > > + * [Script Info] and [V4+ Styles] section, plus the [Events] line and
> > > > + * the Format line following. It shouldn't include any Dialogue line.
> > >
> > >
> > >
> > > > + * - encoding: Set/allocated/freed by user (before avcodec_open())
> > > [...]
> > > > @@ -742,6 +742,8 @@ av_cold int avcodec_close(AVCodecContext *avctx)
> > > > av_freep(&avctx->priv_data);
> > > > if(avctx->codec && avctx->codec->encode)
> > > > av_freep(&avctx->extradata);
> > > > + if (avctx->codec && avctx->codec->encode)
> > > > + av_freep(&avctx->subtitle_header);
> > > > avctx->codec = NULL;
> > > > entangled_thread_counter--;
> > >
> > > doesnt match
> >
> > Right !
> > To be consistent it seems better to let the application free the memory
> > it allocated, so I just removed this av_freep(). Anyway, there was
> > already the needed av_freep() in ffmpeg.
> >
> > > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > > > index 8a08557..a137e69 100644
> > > > --- a/libavformat/utils.c
> > > > +++ b/libavformat/utils.c
> > > > @@ -2022,6 +2022,9 @@ static int has_codec_parameters(AVCodecContext *enc)
> > > > case AVMEDIA_TYPE_VIDEO:
> > > > val = enc->width && enc->pix_fmt != PIX_FMT_NONE;
> > > > break;
> > > > + case AVMEDIA_TYPE_SUBTITLE:
> > > > + val = !!enc->subtitle_header;
> > > > + break;
> > > > default:
> > > > val = 1;
> > > > break;
> > >
> > > This will slow every sub title decoder down that never sets this
> >
> > The best way to fix this is probably to add a new capability flag to
> > codecs which use a subtitle_header, and check for this flag here.
> > Is this OK ?
>
> if subtitle_header is part of the format then its the format (text/ass/bitmap)
> that decides if such header should be expected.
Kind of...
> But if that doesnt work out then a flag could be used too
Indeed, that doesn't work out. The format (text/ass/bitmap) is a
property of the resulting AVSubtitle, not of the AVCodec.
libavformat need this information before opening the codec, so
a flag in AVCodec seems the best solution to me.
> > Aurel
> > ffmpeg.c | 11 +++++++++++
> > libavcodec/avcodec.h | 15 +++++++++++++++
> > libavformat/utils.c | 8 ++++++++
> > 3 files changed, 34 insertions(+)
> > 708fe8c4e44e8f2d2252805a53b8a7633e202579 subtitle_header.diff
> > commit 078a2c04c0970178c729e3cabed3ae7c5d8cb7fa
> > Author: Aurelien Jacobs <aurel at gnuage.org>
> > Date: Thu Aug 5 22:12:37 2010 +0200
> >
> > allow passing subtitles header between decoder and encoder
> >
> > diff --git a/ffmpeg.c b/ffmpeg.c
> > index 4f59b2e..7622ccc 100644
> > --- a/ffmpeg.c
> > +++ b/ffmpeg.c
> > @@ -2365,6 +2365,7 @@ static int transcode(AVFormatContext **output_files,
> > ost = ost_table[i];
> > if (ost->encoding_needed) {
> > AVCodec *codec = i < nb_output_codecs ? output_codecs[i] : NULL;
> > + AVCodecContext *dec = ist_table[ost->source_index]->st->codec;
> > if (!codec)
> > codec = avcodec_find_encoder(ost->st->codec->codec_id);
> > if (!codec) {
> > @@ -2373,6 +2374,15 @@ static int transcode(AVFormatContext **output_files,
> > ret = AVERROR(EINVAL);
> > goto dump_format;
> > }
> > + if (dec->subtitle_header) {
> > + ost->st->codec->subtitle_header = av_malloc(dec->subtitle_header_size);
> > + if (!ost->st->codec->subtitle_header) {
> > + ret = AVERROR(ENOMEM);
> > + goto dump_format;
> > + }
> > + memcpy(ost->st->codec->subtitle_header, dec->subtitle_header, dec->subtitle_header_size);
> > + ost->st->codec->subtitle_header_size = dec->subtitle_header_size;
> > + }
> > if (avcodec_open(ost->st->codec, codec) < 0) {
> > snprintf(error, sizeof(error), "Error while opening encoder for output stream #%d.%d - maybe incorrect parameters such as bit_rate, rate, width or height",
> > ost->file_index, ost->index);
> > @@ -2728,6 +2738,7 @@ static int transcode(AVFormatContext **output_files,
> > }
> > av_fifo_free(ost->fifo); /* works even if fifo is not
> > initialized but set to zero */
> > + av_freep(&ost->st->codec->subtitle_header);
> > av_free(ost->pict_tmp.data[0]);
> > if (ost->video_resample)
> > sws_freeContext(ost->img_resample_ctx);
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index 4bddbaa..af9523b 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -707,6 +707,10 @@ typedef struct RcOverride{
> > * Codec should fill in channel configuration and samplerate instead of container
> > */
> > #define CODEC_CAP_CHANNEL_CONF 0x0400
> > +/**
> > + * Codec is making use of subtitle_header
> > + */
> > +#define CODEC_CAP_SUBTITLE_HEADER 0x0800
> >
> >
> > //The following defines may change, don't expect compatibility if you use them.
> > @@ -2744,6 +2748,17 @@ typedef struct AVCodecContext {
> > * - decoding: unused
> > */
> > int lpc_passes;
> > +
> > + /**
> > + * Header containing style information for text subtitles.
> > + * For SUBTITLE_ASS subtitle type, it should contain the whole ASS
> > + * [Script Info] and [V4+ Styles] section, plus the [Events] line and
> > + * the Format line following. It shouldn't include any Dialogue line.
> > + * - encoding: Set/allocated/freed by user (before avcodec_open())
> > + * - decoding: Set/allocated/freed by libavcodec (by avcodec_open())
> > + */
> > + uint8_t *subtitle_header;
> > + int subtitle_header_size;
> > } AVCodecContext;
> >
> > /**
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index 8a08557..043e251 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -2006,6 +2006,7 @@ static void av_estimate_timings(AVFormatContext *ic, int64_t old_offset)
> >
> > static int has_codec_parameters(AVCodecContext *enc)
> > {
> > + AVCodec *codec = enc->codec;
> > int val;
> > switch(enc->codec_type) {
> > case AVMEDIA_TYPE_AUDIO:
> > @@ -2022,6 +2023,12 @@ static int has_codec_parameters(AVCodecContext *enc)
> > case AVMEDIA_TYPE_VIDEO:
> > val = enc->width && enc->pix_fmt != PIX_FMT_NONE;
> > break;
> > + case AVMEDIA_TYPE_SUBTITLE:
> > + if (!codec)
>
> > + codec = avcodec_find_decoder(enc->codec_id);
>
> that is ugly (and slow)
OK. I droped it and added an AVCodec parameter to the
has_codec_parameters() funtion instead.
Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: subtitle_header.diff
Type: text/x-diff
Size: 6803 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101107/4672e7fa/attachment.diff>
More information about the ffmpeg-devel
mailing list