[FFmpeg-devel] [PATCH] AVFormat: LRC demuxer and muxer

[Derek Buitenhuis]

On 7/9/2014 3:55 PM, Star Brilliant wrote:
>>> +#include "libavutil/bprint.h"
>>> +#include "libavutil/dict.h"
>>> +
>>> +typedef struct LRCContext {
>>> +    FFDemuxSubtitlesQueue q;
>>> +    int ts_offset;
>>
>> Is int enough for a timestamp difference?
> 
> bprint is used for string and buffer manipulation. Again this #include 
> is adapted from assdec.c.
> 
> timestamp difference is far from enough, since LRC comes with arbitrary 
> line order.
> Another reason is repeated line feature. LRC can have single line that 
> is sung multiple times assigned with different timestamps.
> This is why I use FFDemuxSubtitlesQueue to sort those lines.
> 
> ts_offset is a metadata item. By specifying "offset" in metadata, the 
> whole lyrics would shift in timestamps to get sync with different 
> versions of music.
> My implementation does offset shifting on demuxing. So I have to keep a 
> ts_offset member.

You misunderstand what I am asking. int may be 32bits, and timestamps generate 64.
is LRC guaranteed to only have 32bit timestamps?

>>> +        *start = -(mm*60000LL + ss*1000LL + cs*10LL); // just in case negative pts
>>
>> Why are you mangling the timestamps? Negative timestamps are technically valid.
> 
> Negative timestamps are not effective in real LRCs. However, with the 
> feature called "offset" mentioned just above. A normal timestamp can 
> easily become negative.
> Since most players ignore uneffective lines (LRC has no official spec, 
> it is a de-facto standard). So I will let the players drop those 
> negative timestamps, instead of losing data at FFmpeg layer.

Sorry, I misunderstood the code. I thought you we're making it positive (double negative -> positive).

That said, there is still a bug. %d will read negative integers already, you
do not need to check for '-'.

- Derek