Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH v1 1/2] avformat/imfdec: use CPL start timecode if available
@ 2022-08-23  5:10 pal
  2022-08-23  5:10 ` [FFmpeg-devel] [PATCH v1 2/2] avformat/tests/imf: add CPL timecode test pal
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: pal @ 2022-08-23  5:10 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Pierre-Anthony Lemieux

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

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [FFmpeg-devel] [PATCH v1 2/2] avformat/tests/imf: add CPL timecode test
  2022-08-23  5:10 [FFmpeg-devel] [PATCH v1 1/2] avformat/imfdec: use CPL start timecode if available pal
@ 2022-08-23  5:10 ` pal
  2022-09-05 18:06 ` [FFmpeg-devel] [PATCH v1 1/2] avformat/imfdec: use CPL start timecode if available Pierre-Anthony Lemieux
  2022-09-27 12:40 ` Zane van Iperen
  2 siblings, 0 replies; 5+ messages in thread
From: pal @ 2022-08-23  5:10 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Pierre-Anthony Lemieux

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

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH v1 1/2] avformat/imfdec: use CPL start timecode if available
  2022-08-23  5:10 [FFmpeg-devel] [PATCH v1 1/2] avformat/imfdec: use CPL start timecode if available pal
  2022-08-23  5:10 ` [FFmpeg-devel] [PATCH v1 2/2] avformat/tests/imf: add CPL timecode test pal
@ 2022-09-05 18:06 ` Pierre-Anthony Lemieux
  2022-09-27 12:40 ` Zane van Iperen
  2 siblings, 0 replies; 5+ messages in thread
From: Pierre-Anthony Lemieux @ 2022-09-05 18:06 UTC (permalink / raw)
  To: ffmpeg-devel

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
>
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH v1 1/2] avformat/imfdec: use CPL start timecode if available
  2022-08-23  5:10 [FFmpeg-devel] [PATCH v1 1/2] avformat/imfdec: use CPL start timecode if available pal
  2022-08-23  5:10 ` [FFmpeg-devel] [PATCH v1 2/2] avformat/tests/imf: add CPL timecode test pal
  2022-09-05 18:06 ` [FFmpeg-devel] [PATCH v1 1/2] avformat/imfdec: use CPL start timecode if available Pierre-Anthony Lemieux
@ 2022-09-27 12:40 ` Zane van Iperen
  2022-10-02 16:43   ` Pierre-Anthony Lemieux
  2 siblings, 1 reply; 5+ messages in thread
From: Zane van Iperen @ 2022-09-27 12:40 UTC (permalink / raw)
  To: ffmpeg-devel

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?
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH v1 1/2] avformat/imfdec: use CPL start timecode if available
  2022-09-27 12:40 ` Zane van Iperen
@ 2022-10-02 16:43   ` Pierre-Anthony Lemieux
  0 siblings, 0 replies; 5+ messages in thread
From: Pierre-Anthony Lemieux @ 2022-10-02 16:43 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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".
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-10-02 16:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-23  5:10 [FFmpeg-devel] [PATCH v1 1/2] avformat/imfdec: use CPL start timecode if available pal
2022-08-23  5:10 ` [FFmpeg-devel] [PATCH v1 2/2] avformat/tests/imf: add CPL timecode test pal
2022-09-05 18:06 ` [FFmpeg-devel] [PATCH v1 1/2] avformat/imfdec: use CPL start timecode if available Pierre-Anthony Lemieux
2022-09-27 12:40 ` Zane van Iperen
2022-10-02 16:43   ` Pierre-Anthony Lemieux

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git