[FFmpeg-devel] [PATCH 3/3] lavf/srtenc: do not print more line breaks than necessary.

Nicolas George nicolas.george at normalesup.org
Sun Nov 18 15:26:49 CET 2012

Le primidi 21 brumaire, an CCXXI, Clément Bœsch a écrit :
> Some SubRip packets might have a trailing \n, this is now taken into
> account while adding a separator between events to avoid too much line
> breaks.
> Note that events in the FATE tests have more than one line breaks at the
> end.

It makes the code significantly more complex, while assuming things about
the packet format that are still undecided. Would you agree to leave it out
for now, and come back to it when the other points are unresolved? The extra
newlines are not really harmful.

> +    if (write_ts) {
> +        if (pkt->size > 0 && pkt->data[pkt->size - 1] == '\n')
> +            avio_write(avf->pb, "\n", 1);       // add event separator
> +        else if (pkt->size > 1 && !memcmp(pkt->data+pkt->size-2, "\r\n", 2))
> +            avio_write(avf->pb, "\r\n", 2);     // add dos-like event separator
> +        else
> +            avio_write(avf->pb, "\n\n", 2);     // ends event and add separator
> +    }

If I read things correctly, the second branch is unreachable. And testing
the final newline to determine whether we need CRLF or LF is not enough, if
there is no final newline: "- Hi.[\r]\n- Hello."


  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121118/5f0bbf06/attachment.asc>

More information about the ffmpeg-devel mailing list