[FFmpeg-devel] [PATCH] libavformat: libavformat: output cues for each subtitle block in MKV muxer

John Peebles johnpeeb at gmail.com
Sat May 31 05:49:12 CEST 2014


> this diff could be made a bit simpler as well as reducing the risk of
> bugs from applying older patches when data is left a uint8_t *
> and a  uint8_t ** datap is used as function argument

done.


On Fri, May 30, 2014 at 9:49 PM, Michael Niedermayer <michaelni at gmx.at>
wrote:

> On Sun, May 25, 2014 at 10:05:24AM -0400, John Peebles wrote:
> > > This patch doesn't apply on FFmpeg git master.
> > The new version attached should apply.
> >
> > > please do not include re-indent change into the patch
> > done
> >
> > > Can you also provide a sample and command line to reproduce?
> > Sure.
> > (1) Download the attached same-time-script.mkv file. Note that while
> > this file only contains a subtitle track in order to keep the file size
> > down,
> > the bug still happens even if there are other tracks.
> > (2) Run ffmpeg -i same-time-script.ass -c copy -map 0 bad.mkv
> > (3) Inspect the ebml data in bad.mkv. Notice that while there are two
> > subtitle blocks (one for each of two different subtitles), there is only
> a
> > CueTrackPositions element for the first block. There should be one for
> > each block.
> >
> > > Is the issue you are fixing the same as ticket #3149?
> > No, this patch only deals with cues for subtitle tracks, not audio
> tracks.
> >
> >
> > On Sun, May 25, 2014 at 5:20 AM, Carl Eugen Hoyos <cehoyos at ag.or.at>
> wrote:
> >
> > > John Peebles <johnpeeb <at> gmail.com> writes:
> > >
> > > > If you use ffmpeg to remux an mkv file and the file
> > > > has ASS subtitles, only the first subtitle with a
> > > > given timecode will be added to the list of cues.
> > >
> > > Is the issue you are fixing the same as ticket #3149?
> > > If yes, please add "Fixes ticket #3149" to the
> > > commit message.
> > >
> > > Carl Eugen
> > >
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
>
> >  matroskaenc.c |   71
> +++++++++++++++++++++++++++++++---------------------------
> >  1 file changed, 39 insertions(+), 32 deletions(-)
> > 91aa60e6f0c575c5d4a455c991c236252bc041d7
>  0001-libavformat-output-cues-for-each-subtitle-block-in-M.patch
> > From 722516d9e29ae6be2d65d86ba3a0d7c43771abc7 Mon Sep 17 00:00:00 2001
> > From: John Peebles <johnpeeb at gmail.com>
> > Date: Thu, 22 May 2014 03:22:15 -0400
> > Subject: [PATCH] libavformat: output cues for each subtitle block in MKV
> muxer
> >
> > Signed-off-by: John Peebles <johnpeeb at gmail.com>
> > ---
> >  libavformat/matroskaenc.c | 71
> ++++++++++++++++++++++++++---------------------
> >  1 file changed, 39 insertions(+), 32 deletions(-)
> >
> > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> > index 980bf3a..dbb7cae 100644
> > --- a/libavformat/matroskaenc.c
> > +++ b/libavformat/matroskaenc.c
> > @@ -425,8 +425,9 @@ static int mkv_add_cuepoint(mkv_cues *cues, int
> stream, int tracknum, int64_t ts
> >      return 0;
> >  }
> >
> > -static int64_t mkv_write_cues(AVIOContext *pb, mkv_cues *cues,
> mkv_track *tracks, int num_tracks)
> > +static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues,
> mkv_track *tracks, int num_tracks)
> >  {
> > +    AVIOContext *pb = s->pb;
> >      ebml_master cues_element;
> >      int64_t currentpos;
> >      int i, j;
> > @@ -449,7 +450,7 @@ static int64_t mkv_write_cues(AVIOContext *pb,
> mkv_cues *cues, mkv_track *tracks
> >          for (j = 0; j < cues->num_entries - i && entry[j].pts == pts;
> j++) {
> >              int tracknum = entry[j].stream_idx;
> >              av_assert0(tracknum>=0 && tracknum<num_tracks);
> > -            if (tracks[tracknum].has_cue)
> > +            if (tracks[tracknum].has_cue &&
> s->streams[tracknum]->codec->codec_type != AVMEDIA_TYPE_SUBTITLE)
> >                  continue;
> >              tracks[tracknum].has_cue = 1;
> >              track_positions = start_ebml_master(pb,
> MATROSKA_ID_CUETRACKPOSITION, MAX_CUETRACKPOS_SIZE);
> > @@ -1278,26 +1279,29 @@ static int ass_get_duration(const uint8_t *p)
> >  }
> >
> >  #if FF_API_ASS_SSA
> > -static int mkv_write_ass_blocks(AVFormatContext *s, AVIOContext *pb,
> AVPacket *pkt)
> > +/* Writes the contents of pkt to a block, using data starting at *data.
> > + * If pkt corresponds to more than one block, this writes the contents
> of the first block
> > + * and updates *data so it points to the beginning of the data that
> corresponds to
> > + * the next block.
> > + */
> > +static int mkv_write_ass_block(AVFormatContext *s, AVIOContext *pb,
> AVPacket *pkt, uint8_t **data)
> >  {
> >      MatroskaMuxContext *mkv = s->priv_data;
> > -    int i, layer = 0, max_duration = 0, size, line_size, data_size =
> pkt->size;
> > -    uint8_t *start, *end, *data = pkt->data;
> > +    int i, layer = 0, size, line_size, data_size = pkt->size - (*data -
> pkt->data);
> > +    uint8_t *start, *end;
> >      ebml_master blockgroup;
> >      char buffer[2048];
> >
> > -    while (data_size) {
> > -        int duration = ass_get_duration(data);
> > -        max_duration = FFMAX(duration, max_duration);
> > -        end = memchr(data, '\n', data_size);
> > -        size = line_size = end ? end-data+1 : data_size;
> > +        int duration = ass_get_duration(*data);
> > +        end = memchr(*data, '\n', data_size);
> > +        size = line_size = end ? end-*data+1 : data_size;
> >          size -= end ? (end[-1]=='\r')+1 : 0;
> > -        start = data;
> > +        start = *data;
> >          for (i=0; i<3; i++, start++)
> > -            if (!(start = memchr(start, ',', size-(start-data))))
> > -                return max_duration;
> > -        size -= start - data;
> > -        sscanf(data, "Dialogue: %d,", &layer);
> > +            if (!(start = memchr(start, ',', size-(start-*data))))
> > +                return duration;
> > +        size -= start - *data;
> > +        sscanf(*data, "Dialogue: %d,", &layer);
> >          i = snprintf(buffer, sizeof(buffer), "%"PRId64",%d,",
> >                       s->streams[pkt->stream_index]->nb_frames, layer);
> >          size = FFMIN(i+size, sizeof(buffer));
>
> this diff could be made a bit simpler as well as reducing the risk of
> bugs from applying older patches when data is left a uint8_t *
> and a  uint8_t ** datap is used as function argument
>
> also, feel free to add yourself to MAINTAINERs for subtitles in
> matroska
>
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Many that live deserve death. And some that die deserve life. Can you give
> it to them? Then do not be too eager to deal out death in judgement. For
> even the very wise cannot see all ends. -- Gandalf
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-libavformat-output-cues-for-each-subtitle-block-in-M.patch
Type: application/octet-stream
Size: 7785 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140530/8b8ecc91/attachment.obj>


More information about the ffmpeg-devel mailing list