[FFmpeg-devel] Ping Re: [PATCH v1 1/2] avformat/imfdec: use CPL start timecode if available

Pierre-Anthony Lemieux pal at sandflow.com
Sun Sep 25 18:38:59 EEST 2022


Hi all,

I would very much appreciate a review of the following, which was
posted on August 22.

It addresses https://trac.ffmpeg.org/ticket/9842 and works in concert with [2].

Best,

-- Pierre

[1] https://trac.ffmpeg.org/ticket/9842
[2] https://github.com/FFmpeg/FFmpeg/commit/48fa27e77a6d71be89f216cee13ae7e99d2b5003


Now that [1] has been merged,

On Mon, Sep 5, 2022 at 11:06 AM Pierre-Anthony Lemieux <pal at sandflow.com> wrote:
>
> Just a quick ping.
>
> Looking forward to feedback.
>
> This patchset is intended to address https://trac.ffmpeg.org/ticket/9842.
>
>
> On Mon, Aug 22, 2022 at 10:11 PM <pal at sandflow.com> wrote:
> >
> > From: Pierre-Anthony Lemieux <pal at palemieux.com>
> >
> > The IMF CPL contains an optional timecode start address. This patch reads the
> > latter, if present, into the context's timecode metadata parameter.
> > This addresses https://trac.ffmpeg.org/ticket/9842.
> >
> > ---
> >  libavformat/imf.h     |   2 +
> >  libavformat/imf_cpl.c | 109 ++++++++++++++++++++++++++++++++++++++++++
> >  libavformat/imfdec.c  |  11 +++++
> >  3 files changed, 122 insertions(+)
> >
> > diff --git a/libavformat/imf.h b/libavformat/imf.h
> > index 4271cd9582..70ed007312 100644
> > --- a/libavformat/imf.h
> > +++ b/libavformat/imf.h
> > @@ -59,6 +59,7 @@
> >  #include "libavformat/avio.h"
> >  #include "libavutil/rational.h"
> >  #include "libavutil/uuid.h"
> > +#include "libavutil/timecode.h"
> >  #include <libxml/tree.h>
> >
> >  /**
> > @@ -130,6 +131,7 @@ typedef struct FFIMFCPL {
> >      AVUUID id_uuid;                               /**< CompositionPlaylist/Id element */
> >      xmlChar *content_title_utf8;                     /**< CompositionPlaylist/ContentTitle element */
> >      AVRational edit_rate;                            /**< CompositionPlaylist/EditRate element */
> > +    AVTimecode *tc;                                  /**< CompositionPlaylist/CompositionTimecode element */
> >      FFIMFMarkerVirtualTrack *main_markers_track;     /**< Main Marker Virtual Track */
> >      FFIMFTrackFileVirtualTrack *main_image_2d_track; /**< Main Image Virtual Track */
> >      uint32_t main_audio_track_count;                 /**< Number of Main Audio Virtual Tracks */
> > diff --git a/libavformat/imf_cpl.c b/libavformat/imf_cpl.c
> > index 4acc20feee..1868d7db45 100644
> > --- a/libavformat/imf_cpl.c
> > +++ b/libavformat/imf_cpl.c
> > @@ -116,6 +116,25 @@ int ff_imf_xml_read_uint32(xmlNodePtr element, uint32_t *number)
> >      return ret;
> >  }
> >
> > +static int ff_imf_xml_read_boolean(xmlNodePtr element, int *value)
> > +{
> > +    xmlChar *element_text = NULL;
> > +    int ret = 0;
> > +
> > +    element_text = xmlNodeListGetString(element->doc, element->xmlChildrenNode, 1);
> > +
> > +    if (xmlStrcmp(element_text, "true") == 0 || xmlStrcmp(element_text, "1") == 0)
> > +        *value = 1;
> > +    else if (xmlStrcmp(element_text, "false") == 0 || xmlStrcmp(element_text, "0") == 0)
> > +        *value = 0;
> > +    else
> > +        ret = 1;
> > +
> > +    xmlFree(element_text);
> > +
> > +    return ret;
> > +}
> > +
> >  static void imf_base_virtual_track_init(FFIMFBaseVirtualTrack *track)
> >  {
> >      memset(track->id_uuid, 0, sizeof(track->id_uuid));
> > @@ -179,6 +198,90 @@ static int fill_content_title(xmlNodePtr cpl_element, FFIMFCPL *cpl)
> >      return 0;
> >  }
> >
> > +static int digit_to_int(char digit)
> > +{
> > +    if (digit >= '0' && digit <= '9')
> > +        return digit - '0';
> > +    return -1;
> > +}
> > +
> > +/**
> > + * Parses a string that conform to the TimecodeType used in IMF CPL and defined
> > + * in SMPTE ST 2067-3.
> > + * @param[in] s string to parse
> > + * @param[out] tc_comps pointer to an array of 4 integers where the parsed HH,
> > + *                      MM, SS and FF fields of the timecode are returned.
> > + * @return 0 on success, < 0 AVERROR code on error.
> > + */
> > +static int parse_cpl_tc_type(const char *s, int *tc_comps)
> > +{
> > +    if (av_strnlen(s, 11) != 11)
> > +        return AVERROR(EINVAL);
> > +
> > +    for (int i = 0; i < 4; i++) {
> > +        int hi;
> > +        int lo;
> > +
> > +        hi = digit_to_int(s[i * 3]);
> > +        lo = digit_to_int(s[i * 3 + 1]);
> > +
> > +        if (hi == -1 || lo == -1)
> > +            return AVERROR(EINVAL);
> > +
> > +        tc_comps[i] = 10 * hi + lo;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int fill_timecode(xmlNodePtr cpl_element, FFIMFCPL *cpl)
> > +{
> > +    xmlNodePtr tc_element = NULL;
> > +    xmlNodePtr element = NULL;
> > +    xmlChar *tc_str = NULL;
> > +    int df = 0;
> > +    int comps[4];
> > +    int ret = 0;
> > +
> > +    tc_element = ff_imf_xml_get_child_element_by_name(cpl_element, "CompositionTimecode");
> > +    if (!tc_element)
> > +       return 0;
> > +
> > +    element = ff_imf_xml_get_child_element_by_name(tc_element, "TimecodeDropFrame");
> > +    if (!element) {
> > +        av_log(NULL, AV_LOG_ERROR, "CompositionTimecode element is missing\
> > +                                    a TimecodeDropFrame child element\n");
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    if (ff_imf_xml_read_boolean(element, &df)) {
> > +        av_log(NULL, AV_LOG_ERROR, "TimecodeDropFrame element is invalid\n");
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +    element = ff_imf_xml_get_child_element_by_name(tc_element, "TimecodeStartAddress");
> > +    if (!element) {
> > +        av_log(NULL, AV_LOG_ERROR, "CompositionTimecode element is missing\
> > +                                    a TimecodeStartAddress child element\n");
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    tc_str = xmlNodeListGetString(element->doc, element->xmlChildrenNode, 1);
> > +    ret = parse_cpl_tc_type(tc_str, comps);
> > +    xmlFree(tc_str);
> > +    if (ret)
> > +        return ret;
> > +
> > +    cpl->tc = av_malloc(sizeof(AVTimecode));
> > +    if (!cpl->tc)
> > +        return AVERROR(ENOMEM);
> > +    ret = av_timecode_init_from_components(cpl->tc, cpl->edit_rate,
> > +                                           df ? AV_TIMECODE_FLAG_DROPFRAME : 0,
> > +                                           comps[0], comps[1], comps[2], comps[3],
> > +                                           NULL);
> > +
> > +    return ret;
> > +}
> > +
> >  static int fill_edit_rate(xmlNodePtr cpl_element, FFIMFCPL *cpl)
> >  {
> >      xmlNodePtr element = NULL;
> > @@ -682,6 +785,8 @@ int ff_imf_parse_cpl_from_xml_dom(xmlDocPtr doc, FFIMFCPL **cpl)
> >          goto cleanup;
> >      if ((ret = fill_edit_rate(cpl_element, *cpl)))
> >          goto cleanup;
> > +    if ((ret = fill_timecode(cpl_element, *cpl)))
> > +        goto cleanup;
> >      if ((ret = fill_virtual_tracks(cpl_element, *cpl)))
> >          goto cleanup;
> >
> > @@ -731,6 +836,7 @@ static void imf_cpl_init(FFIMFCPL *cpl)
> >      av_uuid_nil(cpl->id_uuid);
> >      cpl->content_title_utf8 = NULL;
> >      cpl->edit_rate = av_make_q(0, 1);
> > +    cpl->tc = NULL;
> >      cpl->main_markers_track = NULL;
> >      cpl->main_image_2d_track = NULL;
> >      cpl->main_audio_track_count = 0;
> > @@ -753,6 +859,9 @@ void ff_imf_cpl_free(FFIMFCPL *cpl)
> >      if (!cpl)
> >          return;
> >
> > +    if (cpl->tc)
> > +        av_freep(&cpl->tc);
> > +
> >      xmlFree(cpl->content_title_utf8);
> >
> >      imf_marker_virtual_track_free(cpl->main_markers_track);
> > diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
> > index 5bbe7a53f8..fdf9a4e87c 100644
> > --- a/libavformat/imfdec.c
> > +++ b/libavformat/imfdec.c
> > @@ -622,6 +622,8 @@ static int imf_read_header(AVFormatContext *s)
> >      IMFContext *c = s->priv_data;
> >      char *asset_map_path;
> >      char *tmp_str;
> > +    AVDictionaryEntry* tcr;
> > +    char tc_buf[AV_TIMECODE_STR_SIZE];
> >      int ret = 0;
> >
> >      c->interrupt_callback = &s->interrupt_callback;
> > @@ -641,6 +643,15 @@ static int imf_read_header(AVFormatContext *s)
> >      if ((ret = ff_imf_parse_cpl(s->pb, &c->cpl)) < 0)
> >          return ret;
> >
> > +    tcr = av_dict_get(s->metadata, "timecode", NULL, 0);
> > +    if (!tcr && c->cpl->tc) {
> > +        ret = av_dict_set(&s->metadata, "timecode",
> > +                          av_timecode_make_string(c->cpl->tc, tc_buf, 0), 0);
> > +        if (ret)
> > +            return ret;
> > +        av_log(s, AV_LOG_INFO, "Setting timecode to IMF CPL timecode %s\n", tc_buf);
> > +    }
> > +
> >      av_log(s,
> >             AV_LOG_DEBUG,
> >             "parsed IMF CPL: " AV_PRI_URN_UUID "\n",
> > --
> > 2.25.1
> >


More information about the ffmpeg-devel mailing list