diff mbox

[FFmpeg-devel,1/7] lavf: add cue sheet demuxer

Message ID 1501569234-29896-1-git-send-email-rodger.combs@gmail.com
State Superseded
Headers show

Commit Message

Rodger Combs Aug. 1, 2017, 6:33 a.m. UTC
---
 Changelog                |   2 +
 doc/demuxers.texi        |   8 ++
 libavformat/Makefile     |   1 +
 libavformat/allformats.c |   1 +
 libavformat/cuedec.c     | 215 +++++++++++++++++++++++++++++++++++++++++++++++
 libavformat/version.h    |   2 +-
 6 files changed, 228 insertions(+), 1 deletion(-)
 create mode 100644 libavformat/cuedec.c

Comments

Hendrik Leppkes Aug. 1, 2017, 6:48 a.m. UTC | #1
On Tue, Aug 1, 2017 at 8:33 AM, Rodger Combs <rodger.combs@gmail.com> wrote:
> ---
>  Changelog                |   2 +
>  doc/demuxers.texi        |   8 ++
>  libavformat/Makefile     |   1 +
>  libavformat/allformats.c |   1 +
>  libavformat/cuedec.c     | 215 +++++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/version.h    |   2 +-
>  6 files changed, 228 insertions(+), 1 deletion(-)
>  create mode 100644 libavformat/cuedec.c
>

Cue Sheets not only support a single data file, but can also contain
metadata for multiple files with one track per file already split (or
even a wild mix of multiple files with multiple tracks per file).
If you don't want to support this, you should probably document this
and perhaps also check in the Cue Sheet parsing if multiple FILE
directives are found, and error out.

- Hendrik
Nicolas George Aug. 1, 2017, 7:58 a.m. UTC | #2
Le quartidi 14 thermidor, an CCXXV, Rodger Combs a écrit :
> ---
>  Changelog                |   2 +
>  doc/demuxers.texi        |   8 ++
>  libavformat/Makefile     |   1 +
>  libavformat/allformats.c |   1 +
>  libavformat/cuedec.c     | 215 +++++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/version.h    |   2 +-
>  6 files changed, 228 insertions(+), 1 deletion(-)
>  create mode 100644 libavformat/cuedec.c
> 
> diff --git a/Changelog b/Changelog
> index 187ae79..6701d30 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -29,6 +29,8 @@ version <next>:
>  - limiter video filter
>  - libvmaf video filter
>  - Dolby E decoder and SMPTE 337M demuxer
> +- Cue sheet demuxer
> +
>  
>  version 3.3:
>  - CrystalHD decoder moved to new decode API
> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> index 29a23d4..7ea4f27 100644
> --- a/doc/demuxers.texi
> +++ b/doc/demuxers.texi
> @@ -244,6 +244,14 @@ file subdir/file-2.wav
>  @end example
>  @end itemize
>  
> +@section cue
> +
> +Cue sheet demuxer.
> +
> +This demuxer reads a cue sheet (text file) and exports its track listing in
> +the form of AVChapters. Packet data is read from the file listed in the sheet.
> +To override the path the packet data is read from, use the @code{url} option.
> +
>  @section flv, live_flv
>  
>  Adobe Flash Video Format demuxer.
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index b0ef82c..4381c42 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -130,6 +130,7 @@ OBJS-$(CONFIG_CDXL_DEMUXER)              += cdxl.o
>  OBJS-$(CONFIG_CINE_DEMUXER)              += cinedec.o
>  OBJS-$(CONFIG_CONCAT_DEMUXER)            += concatdec.o
>  OBJS-$(CONFIG_CRC_MUXER)                 += crcenc.o
> +OBJS-$(CONFIG_CUE_DEMUXER)               += cuedec.o
>  OBJS-$(CONFIG_DATA_DEMUXER)              += rawdec.o
>  OBJS-$(CONFIG_DATA_MUXER)                += rawenc.o
>  OBJS-$(CONFIG_DASH_MUXER)                += dashenc.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 1ebc142..25afa8b 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -96,6 +96,7 @@ static void register_all(void)
>      REGISTER_DEMUXER (CINE,             cine);
>      REGISTER_DEMUXER (CONCAT,           concat);
>      REGISTER_MUXER   (CRC,              crc);
> +    REGISTER_DEMUXER (CUE,              cue);
>      REGISTER_MUXER   (DASH,             dash);
>      REGISTER_MUXDEMUX(DATA,             data);
>      REGISTER_MUXDEMUX(DAUD,             daud);
> diff --git a/libavformat/cuedec.c b/libavformat/cuedec.c
> new file mode 100644
> index 0000000..d0dcac4
> --- /dev/null
> +++ b/libavformat/cuedec.c
> @@ -0,0 +1,215 @@
> +/*
> + * Cue sheet demuxer
> + * Copyright (c) 2016 The FFmpeg Project
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * Cue sheet demuxer
> + * @author Rodger Combs <rodger.combs@gmail.com>
> + */
> +
> +#include "avformat.h"
> +#include "internal.h"
> +#include "subtitles.h"
> +#include "url.h"
> +#include "libavutil/intreadwrite.h"
> +#include "libavutil/avstring.h"
> +#include "libavutil/opt.h"
> +
> +typedef struct CueDemuxContext {
> +    AVClass *class;
> +    char *url;
> +    AVFormatContext *avf;
> +} CueDemuxContext;
> +
> +static int cue_probe(AVProbeData *p)
> +{
> +    const unsigned char *ptr = p->buf;
> +
> +    if (AV_RB24(ptr) == 0xEFBBBF)
> +        ptr += 3;  /* skip UTF-8 BOM */

> +    while (*ptr && strncmp(ptr, "FILE ", 5))
> +        ptr += ff_subtitles_next_line(ptr);
> +    if (!strncmp(ptr, "FILE ", 5))
> +        return AVPROBE_SCORE_MAX - 5;

The duplicated test feels inelegant to me. Better:

    while (*ptr) {
	if (strncmp(...))
	    return ...;
	ptr += ...;
    }

Also, this code matches any text file with a line starting with the word
FILE near the beginning. In other words, it would recognize this very
mail as a cue sheet! I think it needs to be stricter: at least FILE
followed by spaces and a double quote and "TRACK ?? AUDIO" later.

(It would be nice to have a built-in regex compiler that would combine
all similar probe functions into a single finite-state machine.)

> +    return 0;
> +}
> +
> +static char *get_token(char *in)
> +{
> +    char *end;
> +    while (av_isspace(*in))
> +        in++;
> +    if (*in == '"') {
> +        in++;
> +        end = in + strcspn(in, "\"\n\t\r");
> +    } else {
> +        end = in + strcspn(in, " \n\t\r");
> +    }
> +    *end = '\0';
> +    return in;
> +}
> +
> +static int cue_read_header(AVFormatContext *s)
> +{
> +    int ret, i;
> +    CueDemuxContext *cue = s->priv_data;
> +    char line[4096], *ptr;
> +    AVDictionary **meta = &s->metadata;
> +    AVChapter *chap = NULL;
> +    while (ff_get_line(s->pb, line, sizeof(line))) {
> +        ptr = line;
> +        if (AV_RB24(ptr) == 0xEFBBBF)
> +            ptr += 3;  /* skip UTF-8 BOM */
> +        while (*ptr == ' ' || *ptr == '\t')
> +            ptr++;
> +        if (!strncmp(ptr, "REM ", 4)) {
> +            char *end = ptr + strcspn(ptr, "\r\n");
> +            *end = '\0';
> +            av_log(s, AV_LOG_INFO, "Comment: \"%s\"\n", ptr + 4);
> +        } else if (!strncmp(ptr, "TITLE ", 6)) {
> +            ptr = get_token(ptr + 6);
> +            av_dict_set(meta, chap ? "title" : "album", ptr, 0);
> +        } else if (!strncmp(ptr, "PERFORMER ", 10)) {
> +            ptr = get_token(ptr + 10);
> +            av_dict_set(meta, chap ? "artist" : "album_artist", ptr, 0);
> +        } else if (!strncmp(ptr, "FILE ", 5)) {
> +            if (!cue->url || !*cue->url) {
> +                const char *filename = get_token(ptr + 5);
> +                char url[4096] = {0};
> +
> +                if (filename[strcspn(filename, "/\\:")] != 0) {
> +                    av_log(s, AV_LOG_ERROR, "Only bare filenames are allowed in cue FILE directives.\n"
> +                           "To read from '%s', use the 'url' option explicitly.", filename);

> +                    return AVERROR_INVALIDDATA;

AVERROR(EPERM)? Like in concat and hls.

> +                }
> +
> +                av_freep(&cue->url);
> +                ff_make_absolute_url(url, sizeof(url), s->filename, filename);

> +                if (!(cue->url = av_strdup(url)))

If the condition above was met because of !cue->url, then it is ok, but
if it was !*cue->url, then this is leaking.

> +                    return AVERROR(ENOMEM);
> +            }
> +        } else if (!strncmp(ptr, "TRACK ", 6)) {
> +            int index = strtol(ptr + 6, &ptr, 10);
> +            chap = avpriv_new_chapter(s, index, (AVRational){1, 75}, AV_NOPTS_VALUE, AV_NOPTS_VALUE, NULL);
> +            if (!chap)
> +                return AVERROR(ENOMEM);
> +            meta = &chap->metadata;
> +            if ((ret = av_dict_copy(meta, s->metadata, 0)) < 0)
> +                return ret;
> +            av_dict_set_int(meta, "track", index, 0);
> +        } else if (!strncmp(ptr, "INDEX ", 6)) {
> +            int min, sec, frame;

> +            int index = strtol(ptr + 6, &ptr, 10);
> +            if (!chap)
> +                return AVERROR_INVALIDDATA;
> +            if (sscanf(ptr, "%u:%u:%u", &min, &sec, &frame) != 3)
> +                return AVERROR_INVALIDDATA;

You can combine the strtol() and sscanf() into a single sscanf():
sscanf(ptr, "%u %u:%u:%u", ...).

> +            if (index == 1 || chap->start == 0)
> +                chap->start = min * 75 * 60 + sec * 75 + frame;
> +        } else {
> +            av_log(s, AV_LOG_WARNING, "Unknown command: \"%s\"\n", ptr);
> +        }
> +    }
> +
> +    if (!cue->url || !*cue->url)
> +        return AVERROR_INVALIDDATA;
> +
> +    if (!(cue->avf = avformat_alloc_context()))
> +        return AVERROR(ENOMEM);
> +
> +    cue->avf->interrupt_callback = s->interrupt_callback;
> +    if ((ret = ff_copy_whiteblacklists(cue->avf, s)) < 0)
> +        return ret;
> +
> +    if ((ret = avformat_open_input(&cue->avf, cue->url, NULL, NULL)) < 0 ||
> +        (ret = avformat_find_stream_info(cue->avf, NULL)) < 0) {
> +        av_log(s, AV_LOG_ERROR, "Failed to open '%s'\n", cue->url);
> +        avformat_close_input(&cue->avf);
> +        return ret;
> +    }
> +
> +    ff_read_frame_flush(cue->avf);
> +
> +    for (i = 0; i < cue->avf->nb_streams; i++) {
> +        AVStream *st = avformat_new_stream(s, NULL);
> +        AVStream *ist = cue->avf->streams[i];
> +        if (!st)
> +            return AVERROR(ENOMEM);
> +        st->id = i;
> +
> +        avcodec_parameters_copy(st->codecpar, ist->codecpar);
> +
> +        st->disposition = ist->disposition;
> +        avpriv_set_pts_info(st, ist->pts_wrap_bits, ist->time_base.num, ist->time_base.den);
> +        av_copy_packet(&st->attached_pic, &ist->attached_pic);
> +    }
> +
> +    s->duration = cue->avf->duration;
> +
> +    return 0;
> +}
> +
> +static int cue_read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    CueDemuxContext *cue = s->priv_data;
> +    return av_read_frame(cue->avf, pkt);
> +}
> +
> +static int cue_read_seek(AVFormatContext *s, int stream_index,
> +                         int64_t min_ts, int64_t ts, int64_t max_ts, int flags)
> +{
> +    CueDemuxContext *cue = s->priv_data;
> +    return avformat_seek_file(cue->avf, stream_index, min_ts, ts, max_ts, flags);
> +}
> +
> +static int cue_read_close(AVFormatContext *s)
> +{
> +    CueDemuxContext *cue = s->priv_data;
> +    avformat_close_input(&cue->avf);
> +    return 0;
> +}
> +
> +#define OFFSET(x) offsetof(CueDemuxContext, x)
> +#define E AV_OPT_FLAG_DECODING_PARAM
> +static const AVOption options[] = {
> +    { "url",  "override underlying audio location", OFFSET(url), AV_OPT_TYPE_STRING, {.str = ""}, CHAR_MIN, CHAR_MAX, E },
> +    { NULL }
> +};
> +
> +static const AVClass cue_class = {
> +    .class_name = "Cue sheet demuxer",
> +    .item_name  = av_default_item_name,
> +    .option     = options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};
> +
> +AVInputFormat ff_cue_demuxer = {
> +    .name           = "cue",
> +    .long_name      = NULL_IF_CONFIG_SMALL("Cue sheet"),
> +    .extensions     = "cue",
> +    .priv_data_size = sizeof(CueDemuxContext),
> +    .read_probe     = cue_probe,
> +    .read_header    = cue_read_header,
> +    .read_packet    = cue_read_packet,
> +    .read_seek2     = cue_read_seek,
> +    .read_close     = cue_read_close,
> +    .priv_class     = &cue_class,
> +};
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 48b81f2..a8cf4c1 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -32,7 +32,7 @@
>  // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
>  // Also please add any ticket numbers that you believe might be affected here
>  #define LIBAVFORMAT_VERSION_MAJOR  57
> -#define LIBAVFORMAT_VERSION_MINOR  76
> +#define LIBAVFORMAT_VERSION_MINOR  77
>  #define LIBAVFORMAT_VERSION_MICRO 100
>  
>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \

Regards,
Rodger Combs Aug. 2, 2017, 6:28 a.m. UTC | #3
> On Aug 1, 2017, at 02:58, Nicolas George <george@nsup.org> wrote:
> 
> Le quartidi 14 thermidor, an CCXXV, Rodger Combs a écrit :
>> ---
>> Changelog                |   2 +
>> doc/demuxers.texi        |   8 ++
>> libavformat/Makefile     |   1 +
>> libavformat/allformats.c |   1 +
>> libavformat/cuedec.c     | 215 +++++++++++++++++++++++++++++++++++++++++++++++
>> libavformat/version.h    |   2 +-
>> 6 files changed, 228 insertions(+), 1 deletion(-)
>> create mode 100644 libavformat/cuedec.c
>> 
>> diff --git a/Changelog b/Changelog
>> index 187ae79..6701d30 100644
>> --- a/Changelog
>> +++ b/Changelog
>> @@ -29,6 +29,8 @@ version <next>:
>> - limiter video filter
>> - libvmaf video filter
>> - Dolby E decoder and SMPTE 337M demuxer
>> +- Cue sheet demuxer
>> +
>> 
>> version 3.3:
>> - CrystalHD decoder moved to new decode API
>> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
>> index 29a23d4..7ea4f27 100644
>> --- a/doc/demuxers.texi
>> +++ b/doc/demuxers.texi
>> @@ -244,6 +244,14 @@ file subdir/file-2.wav
>> @end example
>> @end itemize
>> 
>> +@section cue
>> +
>> +Cue sheet demuxer.
>> +
>> +This demuxer reads a cue sheet (text file) and exports its track listing in
>> +the form of AVChapters. Packet data is read from the file listed in the sheet.
>> +To override the path the packet data is read from, use the @code{url} option.
>> +
>> @section flv, live_flv
>> 
>> Adobe Flash Video Format demuxer.
>> diff --git a/libavformat/Makefile b/libavformat/Makefile
>> index b0ef82c..4381c42 100644
>> --- a/libavformat/Makefile
>> +++ b/libavformat/Makefile
>> @@ -130,6 +130,7 @@ OBJS-$(CONFIG_CDXL_DEMUXER)              += cdxl.o
>> OBJS-$(CONFIG_CINE_DEMUXER)              += cinedec.o
>> OBJS-$(CONFIG_CONCAT_DEMUXER)            += concatdec.o
>> OBJS-$(CONFIG_CRC_MUXER)                 += crcenc.o
>> +OBJS-$(CONFIG_CUE_DEMUXER)               += cuedec.o
>> OBJS-$(CONFIG_DATA_DEMUXER)              += rawdec.o
>> OBJS-$(CONFIG_DATA_MUXER)                += rawenc.o
>> OBJS-$(CONFIG_DASH_MUXER)                += dashenc.o
>> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
>> index 1ebc142..25afa8b 100644
>> --- a/libavformat/allformats.c
>> +++ b/libavformat/allformats.c
>> @@ -96,6 +96,7 @@ static void register_all(void)
>>     REGISTER_DEMUXER (CINE,             cine);
>>     REGISTER_DEMUXER (CONCAT,           concat);
>>     REGISTER_MUXER   (CRC,              crc);
>> +    REGISTER_DEMUXER (CUE,              cue);
>>     REGISTER_MUXER   (DASH,             dash);
>>     REGISTER_MUXDEMUX(DATA,             data);
>>     REGISTER_MUXDEMUX(DAUD,             daud);
>> diff --git a/libavformat/cuedec.c b/libavformat/cuedec.c
>> new file mode 100644
>> index 0000000..d0dcac4
>> --- /dev/null
>> +++ b/libavformat/cuedec.c
>> @@ -0,0 +1,215 @@
>> +/*
>> + * Cue sheet demuxer
>> + * Copyright (c) 2016 The FFmpeg Project
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +
>> +/**
>> + * @file
>> + * Cue sheet demuxer
>> + * @author Rodger Combs <rodger.combs@gmail.com>
>> + */
>> +
>> +#include "avformat.h"
>> +#include "internal.h"
>> +#include "subtitles.h"
>> +#include "url.h"
>> +#include "libavutil/intreadwrite.h"
>> +#include "libavutil/avstring.h"
>> +#include "libavutil/opt.h"
>> +
>> +typedef struct CueDemuxContext {
>> +    AVClass *class;
>> +    char *url;
>> +    AVFormatContext *avf;
>> +} CueDemuxContext;
>> +
>> +static int cue_probe(AVProbeData *p)
>> +{
>> +    const unsigned char *ptr = p->buf;
>> +
>> +    if (AV_RB24(ptr) == 0xEFBBBF)
>> +        ptr += 3;  /* skip UTF-8 BOM */
> 
>> +    while (*ptr && strncmp(ptr, "FILE ", 5))
>> +        ptr += ff_subtitles_next_line(ptr);
>> +    if (!strncmp(ptr, "FILE ", 5))
>> +        return AVPROBE_SCORE_MAX - 5;
> 
> The duplicated test feels inelegant to me. Better:
> 
>    while (*ptr) {
> 	if (strncmp(...))
> 	    return ...;
> 	ptr += ...;
>    }

Moving in the direction of this format in my new version.

> 
> Also, this code matches any text file with a line starting with the word
> FILE near the beginning. In other words, it would recognize this very
> mail as a cue sheet! I think it needs to be stricter: at least FILE
> followed by spaces and a double quote and "TRACK ?? AUDIO" later.

Done.

> 
> (It would be nice to have a built-in regex compiler that would combine
> all similar probe functions into a single finite-state machine.)
> 
>> +    return 0;
>> +}
>> +
>> +static char *get_token(char *in)
>> +{
>> +    char *end;
>> +    while (av_isspace(*in))
>> +        in++;
>> +    if (*in == '"') {
>> +        in++;
>> +        end = in + strcspn(in, "\"\n\t\r");
>> +    } else {
>> +        end = in + strcspn(in, " \n\t\r");
>> +    }
>> +    *end = '\0';
>> +    return in;
>> +}
>> +
>> +static int cue_read_header(AVFormatContext *s)
>> +{
>> +    int ret, i;
>> +    CueDemuxContext *cue = s->priv_data;
>> +    char line[4096], *ptr;
>> +    AVDictionary **meta = &s->metadata;
>> +    AVChapter *chap = NULL;
>> +    while (ff_get_line(s->pb, line, sizeof(line))) {
>> +        ptr = line;
>> +        if (AV_RB24(ptr) == 0xEFBBBF)
>> +            ptr += 3;  /* skip UTF-8 BOM */
>> +        while (*ptr == ' ' || *ptr == '\t')
>> +            ptr++;
>> +        if (!strncmp(ptr, "REM ", 4)) {
>> +            char *end = ptr + strcspn(ptr, "\r\n");
>> +            *end = '\0';
>> +            av_log(s, AV_LOG_INFO, "Comment: \"%s\"\n", ptr + 4);
>> +        } else if (!strncmp(ptr, "TITLE ", 6)) {
>> +            ptr = get_token(ptr + 6);
>> +            av_dict_set(meta, chap ? "title" : "album", ptr, 0);
>> +        } else if (!strncmp(ptr, "PERFORMER ", 10)) {
>> +            ptr = get_token(ptr + 10);
>> +            av_dict_set(meta, chap ? "artist" : "album_artist", ptr, 0);
>> +        } else if (!strncmp(ptr, "FILE ", 5)) {
>> +            if (!cue->url || !*cue->url) {
>> +                const char *filename = get_token(ptr + 5);
>> +                char url[4096] = {0};
>> +
>> +                if (filename[strcspn(filename, "/\\:")] != 0) {
>> +                    av_log(s, AV_LOG_ERROR, "Only bare filenames are allowed in cue FILE directives.\n"
>> +                           "To read from '%s', use the 'url' option explicitly.", filename);
> 
>> +                    return AVERROR_INVALIDDATA;
> 
> AVERROR(EPERM)? Like in concat and hls.

Done.

> 
>> +                }
>> +
>> +                av_freep(&cue->url);
>> +                ff_make_absolute_url(url, sizeof(url), s->filename, filename);
> 
>> +                if (!(cue->url = av_strdup(url)))
> 
> If the condition above was met because of !cue->url, then it is ok, but
> if it was !*cue->url, then this is leaking.

Note the preceding av_freep().

> 
>> +                    return AVERROR(ENOMEM);
>> +            }
>> +        } else if (!strncmp(ptr, "TRACK ", 6)) {
>> +            int index = strtol(ptr + 6, &ptr, 10);
>> +            chap = avpriv_new_chapter(s, index, (AVRational){1, 75}, AV_NOPTS_VALUE, AV_NOPTS_VALUE, NULL);
>> +            if (!chap)
>> +                return AVERROR(ENOMEM);
>> +            meta = &chap->metadata;
>> +            if ((ret = av_dict_copy(meta, s->metadata, 0)) < 0)
>> +                return ret;
>> +            av_dict_set_int(meta, "track", index, 0);
>> +        } else if (!strncmp(ptr, "INDEX ", 6)) {
>> +            int min, sec, frame;
> 
>> +            int index = strtol(ptr + 6, &ptr, 10);
>> +            if (!chap)
>> +                return AVERROR_INVALIDDATA;
>> +            if (sscanf(ptr, "%u:%u:%u", &min, &sec, &frame) != 3)
>> +                return AVERROR_INVALIDDATA;
> 
> You can combine the strtol() and sscanf() into a single sscanf():
> sscanf(ptr, "%u %u:%u:%u", ...).

Done.

> 
>> +            if (index == 1 || chap->start == 0)
>> +                chap->start = min * 75 * 60 + sec * 75 + frame;
>> +        } else {
>> +            av_log(s, AV_LOG_WARNING, "Unknown command: \"%s\"\n", ptr);
>> +        }
>> +    }
>> +
>> +    if (!cue->url || !*cue->url)
>> +        return AVERROR_INVALIDDATA;
>> +
>> +    if (!(cue->avf = avformat_alloc_context()))
>> +        return AVERROR(ENOMEM);
>> +
>> +    cue->avf->interrupt_callback = s->interrupt_callback;
>> +    if ((ret = ff_copy_whiteblacklists(cue->avf, s)) < 0)
>> +        return ret;
>> +
>> +    if ((ret = avformat_open_input(&cue->avf, cue->url, NULL, NULL)) < 0 ||
>> +        (ret = avformat_find_stream_info(cue->avf, NULL)) < 0) {
>> +        av_log(s, AV_LOG_ERROR, "Failed to open '%s'\n", cue->url);
>> +        avformat_close_input(&cue->avf);
>> +        return ret;
>> +    }
>> +
>> +    ff_read_frame_flush(cue->avf);
>> +
>> +    for (i = 0; i < cue->avf->nb_streams; i++) {
>> +        AVStream *st = avformat_new_stream(s, NULL);
>> +        AVStream *ist = cue->avf->streams[i];
>> +        if (!st)
>> +            return AVERROR(ENOMEM);
>> +        st->id = i;
>> +
>> +        avcodec_parameters_copy(st->codecpar, ist->codecpar);
>> +
>> +        st->disposition = ist->disposition;
>> +        avpriv_set_pts_info(st, ist->pts_wrap_bits, ist->time_base.num, ist->time_base.den);
>> +        av_copy_packet(&st->attached_pic, &ist->attached_pic);
>> +    }
>> +
>> +    s->duration = cue->avf->duration;
>> +
>> +    return 0;
>> +}
>> +
>> +static int cue_read_packet(AVFormatContext *s, AVPacket *pkt)
>> +{
>> +    CueDemuxContext *cue = s->priv_data;
>> +    return av_read_frame(cue->avf, pkt);
>> +}
>> +
>> +static int cue_read_seek(AVFormatContext *s, int stream_index,
>> +                         int64_t min_ts, int64_t ts, int64_t max_ts, int flags)
>> +{
>> +    CueDemuxContext *cue = s->priv_data;
>> +    return avformat_seek_file(cue->avf, stream_index, min_ts, ts, max_ts, flags);
>> +}
>> +
>> +static int cue_read_close(AVFormatContext *s)
>> +{
>> +    CueDemuxContext *cue = s->priv_data;
>> +    avformat_close_input(&cue->avf);
>> +    return 0;
>> +}
>> +
>> +#define OFFSET(x) offsetof(CueDemuxContext, x)
>> +#define E AV_OPT_FLAG_DECODING_PARAM
>> +static const AVOption options[] = {
>> +    { "url",  "override underlying audio location", OFFSET(url), AV_OPT_TYPE_STRING, {.str = ""}, CHAR_MIN, CHAR_MAX, E },
>> +    { NULL }
>> +};
>> +
>> +static const AVClass cue_class = {
>> +    .class_name = "Cue sheet demuxer",
>> +    .item_name  = av_default_item_name,
>> +    .option     = options,
>> +    .version    = LIBAVUTIL_VERSION_INT,
>> +};
>> +
>> +AVInputFormat ff_cue_demuxer = {
>> +    .name           = "cue",
>> +    .long_name      = NULL_IF_CONFIG_SMALL("Cue sheet"),
>> +    .extensions     = "cue",
>> +    .priv_data_size = sizeof(CueDemuxContext),
>> +    .read_probe     = cue_probe,
>> +    .read_header    = cue_read_header,
>> +    .read_packet    = cue_read_packet,
>> +    .read_seek2     = cue_read_seek,
>> +    .read_close     = cue_read_close,
>> +    .priv_class     = &cue_class,
>> +};
>> diff --git a/libavformat/version.h b/libavformat/version.h
>> index 48b81f2..a8cf4c1 100644
>> --- a/libavformat/version.h
>> +++ b/libavformat/version.h
>> @@ -32,7 +32,7 @@
>> // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
>> // Also please add any ticket numbers that you believe might be affected here
>> #define LIBAVFORMAT_VERSION_MAJOR  57
>> -#define LIBAVFORMAT_VERSION_MINOR  76
>> +#define LIBAVFORMAT_VERSION_MINOR  77
>> #define LIBAVFORMAT_VERSION_MICRO 100
>> 
>> #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> 
> Regards,
> 
> -- 
>  Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
Reino Wijnsma Aug. 23, 2019, 10:46 p.m. UTC | #4
Hello ffmpeg-devel,

I've read through https://patchwork.ffmpeg.org/patch/4587/, https://ffmpeg.org/pipermail/ffmpeg-devel/2017-August/214360.html and all its replies. The discussion suddenly just stops, the path dropped and I can't seem to find the reason why. Can anyone tell me why this patch hasn't been applied? I was just wondering.

-- Reino
diff mbox

Patch

diff --git a/Changelog b/Changelog
index 187ae79..6701d30 100644
--- a/Changelog
+++ b/Changelog
@@ -29,6 +29,8 @@  version <next>:
 - limiter video filter
 - libvmaf video filter
 - Dolby E decoder and SMPTE 337M demuxer
+- Cue sheet demuxer
+
 
 version 3.3:
 - CrystalHD decoder moved to new decode API
diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index 29a23d4..7ea4f27 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -244,6 +244,14 @@  file subdir/file-2.wav
 @end example
 @end itemize
 
+@section cue
+
+Cue sheet demuxer.
+
+This demuxer reads a cue sheet (text file) and exports its track listing in
+the form of AVChapters. Packet data is read from the file listed in the sheet.
+To override the path the packet data is read from, use the @code{url} option.
+
 @section flv, live_flv
 
 Adobe Flash Video Format demuxer.
diff --git a/libavformat/Makefile b/libavformat/Makefile
index b0ef82c..4381c42 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -130,6 +130,7 @@  OBJS-$(CONFIG_CDXL_DEMUXER)              += cdxl.o
 OBJS-$(CONFIG_CINE_DEMUXER)              += cinedec.o
 OBJS-$(CONFIG_CONCAT_DEMUXER)            += concatdec.o
 OBJS-$(CONFIG_CRC_MUXER)                 += crcenc.o
+OBJS-$(CONFIG_CUE_DEMUXER)               += cuedec.o
 OBJS-$(CONFIG_DATA_DEMUXER)              += rawdec.o
 OBJS-$(CONFIG_DATA_MUXER)                += rawenc.o
 OBJS-$(CONFIG_DASH_MUXER)                += dashenc.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index 1ebc142..25afa8b 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -96,6 +96,7 @@  static void register_all(void)
     REGISTER_DEMUXER (CINE,             cine);
     REGISTER_DEMUXER (CONCAT,           concat);
     REGISTER_MUXER   (CRC,              crc);
+    REGISTER_DEMUXER (CUE,              cue);
     REGISTER_MUXER   (DASH,             dash);
     REGISTER_MUXDEMUX(DATA,             data);
     REGISTER_MUXDEMUX(DAUD,             daud);
diff --git a/libavformat/cuedec.c b/libavformat/cuedec.c
new file mode 100644
index 0000000..d0dcac4
--- /dev/null
+++ b/libavformat/cuedec.c
@@ -0,0 +1,215 @@ 
+/*
+ * Cue sheet demuxer
+ * Copyright (c) 2016 The FFmpeg Project
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * Cue sheet demuxer
+ * @author Rodger Combs <rodger.combs@gmail.com>
+ */
+
+#include "avformat.h"
+#include "internal.h"
+#include "subtitles.h"
+#include "url.h"
+#include "libavutil/intreadwrite.h"
+#include "libavutil/avstring.h"
+#include "libavutil/opt.h"
+
+typedef struct CueDemuxContext {
+    AVClass *class;
+    char *url;
+    AVFormatContext *avf;
+} CueDemuxContext;
+
+static int cue_probe(AVProbeData *p)
+{
+    const unsigned char *ptr = p->buf;
+
+    if (AV_RB24(ptr) == 0xEFBBBF)
+        ptr += 3;  /* skip UTF-8 BOM */
+    while (*ptr && strncmp(ptr, "FILE ", 5))
+        ptr += ff_subtitles_next_line(ptr);
+    if (!strncmp(ptr, "FILE ", 5))
+        return AVPROBE_SCORE_MAX - 5;
+    return 0;
+}
+
+static char *get_token(char *in)
+{
+    char *end;
+    while (av_isspace(*in))
+        in++;
+    if (*in == '"') {
+        in++;
+        end = in + strcspn(in, "\"\n\t\r");
+    } else {
+        end = in + strcspn(in, " \n\t\r");
+    }
+    *end = '\0';
+    return in;
+}
+
+static int cue_read_header(AVFormatContext *s)
+{
+    int ret, i;
+    CueDemuxContext *cue = s->priv_data;
+    char line[4096], *ptr;
+    AVDictionary **meta = &s->metadata;
+    AVChapter *chap = NULL;
+    while (ff_get_line(s->pb, line, sizeof(line))) {
+        ptr = line;
+        if (AV_RB24(ptr) == 0xEFBBBF)
+            ptr += 3;  /* skip UTF-8 BOM */
+        while (*ptr == ' ' || *ptr == '\t')
+            ptr++;
+        if (!strncmp(ptr, "REM ", 4)) {
+            char *end = ptr + strcspn(ptr, "\r\n");
+            *end = '\0';
+            av_log(s, AV_LOG_INFO, "Comment: \"%s\"\n", ptr + 4);
+        } else if (!strncmp(ptr, "TITLE ", 6)) {
+            ptr = get_token(ptr + 6);
+            av_dict_set(meta, chap ? "title" : "album", ptr, 0);
+        } else if (!strncmp(ptr, "PERFORMER ", 10)) {
+            ptr = get_token(ptr + 10);
+            av_dict_set(meta, chap ? "artist" : "album_artist", ptr, 0);
+        } else if (!strncmp(ptr, "FILE ", 5)) {
+            if (!cue->url || !*cue->url) {
+                const char *filename = get_token(ptr + 5);
+                char url[4096] = {0};
+
+                if (filename[strcspn(filename, "/\\:")] != 0) {
+                    av_log(s, AV_LOG_ERROR, "Only bare filenames are allowed in cue FILE directives.\n"
+                           "To read from '%s', use the 'url' option explicitly.", filename);
+                    return AVERROR_INVALIDDATA;
+                }
+
+                av_freep(&cue->url);
+                ff_make_absolute_url(url, sizeof(url), s->filename, filename);
+                if (!(cue->url = av_strdup(url)))
+                    return AVERROR(ENOMEM);
+            }
+        } else if (!strncmp(ptr, "TRACK ", 6)) {
+            int index = strtol(ptr + 6, &ptr, 10);
+            chap = avpriv_new_chapter(s, index, (AVRational){1, 75}, AV_NOPTS_VALUE, AV_NOPTS_VALUE, NULL);
+            if (!chap)
+                return AVERROR(ENOMEM);
+            meta = &chap->metadata;
+            if ((ret = av_dict_copy(meta, s->metadata, 0)) < 0)
+                return ret;
+            av_dict_set_int(meta, "track", index, 0);
+        } else if (!strncmp(ptr, "INDEX ", 6)) {
+            int min, sec, frame;
+            int index = strtol(ptr + 6, &ptr, 10);
+            if (!chap)
+                return AVERROR_INVALIDDATA;
+            if (sscanf(ptr, "%u:%u:%u", &min, &sec, &frame) != 3)
+                return AVERROR_INVALIDDATA;
+            if (index == 1 || chap->start == 0)
+                chap->start = min * 75 * 60 + sec * 75 + frame;
+        } else {
+            av_log(s, AV_LOG_WARNING, "Unknown command: \"%s\"\n", ptr);
+        }
+    }
+
+    if (!cue->url || !*cue->url)
+        return AVERROR_INVALIDDATA;
+
+    if (!(cue->avf = avformat_alloc_context()))
+        return AVERROR(ENOMEM);
+
+    cue->avf->interrupt_callback = s->interrupt_callback;
+    if ((ret = ff_copy_whiteblacklists(cue->avf, s)) < 0)
+        return ret;
+
+    if ((ret = avformat_open_input(&cue->avf, cue->url, NULL, NULL)) < 0 ||
+        (ret = avformat_find_stream_info(cue->avf, NULL)) < 0) {
+        av_log(s, AV_LOG_ERROR, "Failed to open '%s'\n", cue->url);
+        avformat_close_input(&cue->avf);
+        return ret;
+    }
+
+    ff_read_frame_flush(cue->avf);
+
+    for (i = 0; i < cue->avf->nb_streams; i++) {
+        AVStream *st = avformat_new_stream(s, NULL);
+        AVStream *ist = cue->avf->streams[i];
+        if (!st)
+            return AVERROR(ENOMEM);
+        st->id = i;
+
+        avcodec_parameters_copy(st->codecpar, ist->codecpar);
+
+        st->disposition = ist->disposition;
+        avpriv_set_pts_info(st, ist->pts_wrap_bits, ist->time_base.num, ist->time_base.den);
+        av_copy_packet(&st->attached_pic, &ist->attached_pic);
+    }
+
+    s->duration = cue->avf->duration;
+
+    return 0;
+}
+
+static int cue_read_packet(AVFormatContext *s, AVPacket *pkt)
+{
+    CueDemuxContext *cue = s->priv_data;
+    return av_read_frame(cue->avf, pkt);
+}
+
+static int cue_read_seek(AVFormatContext *s, int stream_index,
+                         int64_t min_ts, int64_t ts, int64_t max_ts, int flags)
+{
+    CueDemuxContext *cue = s->priv_data;
+    return avformat_seek_file(cue->avf, stream_index, min_ts, ts, max_ts, flags);
+}
+
+static int cue_read_close(AVFormatContext *s)
+{
+    CueDemuxContext *cue = s->priv_data;
+    avformat_close_input(&cue->avf);
+    return 0;
+}
+
+#define OFFSET(x) offsetof(CueDemuxContext, x)
+#define E AV_OPT_FLAG_DECODING_PARAM
+static const AVOption options[] = {
+    { "url",  "override underlying audio location", OFFSET(url), AV_OPT_TYPE_STRING, {.str = ""}, CHAR_MIN, CHAR_MAX, E },
+    { NULL }
+};
+
+static const AVClass cue_class = {
+    .class_name = "Cue sheet demuxer",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
+AVInputFormat ff_cue_demuxer = {
+    .name           = "cue",
+    .long_name      = NULL_IF_CONFIG_SMALL("Cue sheet"),
+    .extensions     = "cue",
+    .priv_data_size = sizeof(CueDemuxContext),
+    .read_probe     = cue_probe,
+    .read_header    = cue_read_header,
+    .read_packet    = cue_read_packet,
+    .read_seek2     = cue_read_seek,
+    .read_close     = cue_read_close,
+    .priv_class     = &cue_class,
+};
diff --git a/libavformat/version.h b/libavformat/version.h
index 48b81f2..a8cf4c1 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@ 
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  57
-#define LIBAVFORMAT_VERSION_MINOR  76
+#define LIBAVFORMAT_VERSION_MINOR  77
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \