[FFmpeg-devel] [PATCH] lavc/srtenc: do not add trailing line breaks with SubRip.

[Clément Bœsch]

On Sun, Dec 30, 2012 at 02:55:15PM +0100, Nicolas George wrote:
> Le decadi 10 nivôse, an CCXXI, Clement Boesch a écrit :
> > The SRT muxer is reponsible for separating events with two line breaks,
> > there is no need to add more than necessary. Similarly, other muxers
> > (such as Matroska) are not supposed to add line breaks at the end of the
> > payload.
> > ---
> > To be applied on top of the 2 previous srtdec patches.
> > ---
> >  libavcodec/srtenc.c          | 5 ++++-
> >  tests/ref/fate/sub-subripenc | 2 +-
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/srtenc.c b/libavcodec/srtenc.c
> > index 739645b..d72d338 100644
> > --- a/libavcodec/srtenc.c
> > +++ b/libavcodec/srtenc.c
> > @@ -218,8 +218,11 @@ static void srt_move_cb(void *priv, int x1, int y1, int x2, int y2,
> >  
> >  static void srt_end_cb(void *priv)
> >  {
> > +    SRTContext *s = priv;
> > +
> >      srt_stack_push_pop(priv, 0, 1);
> > -    srt_print(priv, "\r\n\r\n");
> > +    if (s->avctx->codec->id == AV_CODEC_ID_SRT)
> > +        srt_print(priv, "\r\n\r\n");
> 
> IIRC, AV_CODEC_ID_SRT is the legacy codec-id with timing information in the
> packet. Am I wrong?
> 

Yes, we have a few exceptions in the code already. I kept the behaviour
for the legacy code.

> A small comment to remind that to the reader would be nice.
> 

Added in another commit, just like with lavc/srtdec.

> >  }
> >  
> >  static const ASSCodesCallbacks srt_callbacks = {
> > diff --git a/tests/ref/fate/sub-subripenc b/tests/ref/fate/sub-subripenc
> > index 7daa4f5..9666e9b 100644
> > --- a/tests/ref/fate/sub-subripenc
> > +++ b/tests/ref/fate/sub-subripenc
> > @@ -1 +1 @@
> > -bd520f85238abf9df292374aed54681a
> > +b7cb0eeb34af0da364e29b238f0634ae
> 
> Otherwise looks good to me.
> 

Pushed, thanks.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121230/43893ef8/attachment.asc>