[PATCH v1 1/2] avformat/imfdec: use CPL start timecode if available
From: Pierre-Anthony Lemieux <pal@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
From: Pierre-Anthony Lemieux <pal@palemieux.com> --- libavformat/tests/imf.c | 7 +++++++ tests/ref/fate/imf | 1 + 2 files changed, 8 insertions(+) diff --git a/libavformat/tests/imf.c b/libavformat/tests/imf.c index e65629ccbc..d42295dc29 100644 --- a/libavformat/tests/imf.c +++ b/libavformat/tests/imf.c @@ -70,6 +70,11 @@ const char *cpl_doc = " <Id>urn:uuid:8e097bb0-cff7-4969-a692-bad47bfb528f</Id>" " </EssenceDescriptor>" "</EssenceDescriptorList>" + "<CompositionTimecode>" + "<TimecodeDropFrame>false</TimecodeDropFrame>" + "<TimecodeRate>24</TimecodeRate>" + "<TimecodeStartAddress>02:10:01.23</TimecodeStartAddress>" + "</CompositionTimecode>" "<EditRate>24000 1001</EditRate>" "<SegmentList>" "<Segment>" @@ -288,6 +293,7 @@ static int test_cpl_parsing(void) { xmlDocPtr doc; FFIMFCPL *cpl; + char tc_buf[AV_TIMECODE_STR_SIZE]; int ret; doc = xmlReadMemory(cpl_doc, strlen(cpl_doc), NULL, NULL, 0); @@ -306,6 +312,7 @@ static int test_cpl_parsing(void) printf("%s\n", cpl->content_title_utf8); printf(AV_PRI_URN_UUID "\n", AV_UUID_ARG(cpl->id_uuid)); printf("%i %i\n", cpl->edit_rate.num, cpl->edit_rate.den); + printf("%s\n", av_timecode_make_string(cpl->tc, tc_buf, 0)); printf("Marker resource count: %" PRIu32 "\n", cpl->main_markers_track->resource_count); for (uint32_t i = 0; i < cpl->main_markers_track->resource_count; i++) { diff --git a/tests/ref/fate/imf b/tests/ref/fate/imf index 90b461dc5d..5093167bc7 100644 --- a/tests/ref/fate/imf +++ b/tests/ref/fate/imf @@ -1,6 +1,7 @@ FFMPEG sample content urn:uuid:8713c020-2489-45f5-a9f7-87be539e20b5 24000 1001 +02:10:01:23 Marker resource count: 2 Marker resource 0 Marker 0 -- 2.25.1
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@sandflow.com> wrote:
From: Pierre-Anthony Lemieux <pal@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
Looks mostly ok from a cursory glance, just one minor nit. On 23/8/22 15:10, pal@sandflow.com wrote:
+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); +
No need for "element_text = NULL".
+static int digit_to_int(char digit) +{ + if (digit >= '0' && digit <= '9') + return digit - '0'; + return -1; +} +
I feel like there should be a av_* helper for this, but apparently there isn't. Maybe it's worth adding one in a future patch? av_isdigit() is in avstring.h, so perhaps there?
On Tue, Sep 27, 2022 at 5:40 AM Zane van Iperen <zane@zanevaniperen.com> wrote:
Looks mostly ok from a cursory glance, just one minor nit.
On 23/8/22 15:10, pal@sandflow.com wrote:
+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); +
No need for "element_text = NULL".
Addressed at v2.
+static int digit_to_int(char digit) +{ + if (digit >= '0' && digit <= '9') + return digit - '0'; + return -1; +} +
I feel like there should be a av_* helper for this, but apparently there isn't. Maybe it's worth adding one in a future patch? av_isdigit() is in avstring.h, so perhaps there?
Ok. This probably requires more planning since the idioms `x >= '0' && x <= '9'` and `x >= 'a' && x <= 'z'` are used in many places.
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
participants (3)
-
pal@sandflow.com -
Pierre-Anthony Lemieux -
Zane van Iperen