[FFmpeg-devel] [PATCH 1/2] avformat/imf: fix invalid resource handling

[FFmpeg-devel] [PATCH 1/2] avformat/imf: fix invalid resource handling

[pal]

From: Pierre-Anthony Lemieux <pal@palemieux.com>

---
 libavformat/imf_cpl.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/libavformat/imf_cpl.c b/libavformat/imf_cpl.c
index ad84a68b13..a7cf5fa360 100644
--- a/libavformat/imf_cpl.c
+++ b/libavformat/imf_cpl.c
@@ -608,11 +608,10 @@ static int push_main_audio_sequence(xmlNodePtr audio_sequence_elem, FFIMFCPL *cp
         ret = fill_trackfile_resource(resource_elem,
                                       &vt->resources[vt->resource_count],
                                       cpl);
-        vt->resource_count++;
-        if (ret) {
+        if (ret)
             av_log(NULL, AV_LOG_ERROR, "Invalid Resource\n");
-            continue;
-        }
+        else
+            vt->resource_count++;
 
         resource_elem = xmlNextElementSibling(resource_elem);
     }
@@ -691,11 +690,10 @@ static int push_main_image_2d_sequence(xmlNodePtr image_sequence_elem, FFIMFCPL
         ret = fill_trackfile_resource(resource_elem,
                                       &cpl->main_image_2d_track->resources[cpl->main_image_2d_track->resource_count],
                                       cpl);
-        cpl->main_image_2d_track->resource_count++;
-        if (ret) {
+        if (ret)
             av_log(NULL, AV_LOG_ERROR, "Invalid Resource\n");
-            continue;
-        }
+        else
+            cpl->main_image_2d_track->resource_count++;
 
         resource_elem = xmlNextElementSibling(resource_elem);
     }
-- 
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".

[FFmpeg-devel] [PATCH 2/2] avformat/tests/imf: add invalid resource test

[pal]

From: Pierre-Anthony Lemieux <pal@palemieux.com>

---
 libavformat/tests/imf.c | 65 +++++++++++++++++++++++++++++++++++++++++
 tests/ref/fate/imf      |  2 ++
 2 files changed, 67 insertions(+)

diff --git a/libavformat/tests/imf.c b/libavformat/tests/imf.c
index 2cacb43f47..cfd84fb8c8 100644
--- a/libavformat/tests/imf.c
+++ b/libavformat/tests/imf.c
@@ -218,6 +218,45 @@ const char *cpl_doc =
     "</SegmentList>"
     "</CompositionPlaylist>";
 
+    const char *cpl_bad_resource_doc =
+    "<CompositionPlaylist xmlns=\"http://www.smpte-ra.org/schemas/2067-3/2016\""
+    " xmlns:cc=\"http://www.smpte-ra.org/schemas/2067-2/2016\""
+    " xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\">"
+    "<Id>urn:uuid:8713c020-2489-45f5-a9f7-87be539e20b5</Id>"
+    "<IssueDate>2021-07-13T17:06:22Z</IssueDate>"
+    "<Creator language=\"en\">FFMPEG</Creator>"
+    "<ContentTitle>FFMPEG sample content</ContentTitle>"
+    "<EssenceDescriptorList>"
+    "  <EssenceDescriptor>"
+    "    <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>"
+    "<Id>urn:uuid:81fed4e5-9722-400a-b9d1-7f2bd21df4b6</Id>"
+    "<SequenceList>"
+    "<cc:MainImageSequence>"
+    "<Id>urn:uuid:6ae100b0-92d1-41be-9321-85e0933dfc42</Id>"
+    "<TrackId>urn:uuid:e8ef9653-565c-479c-8039-82d4547973c5</TrackId>"
+    "<ResourceList>"
+    "<Resource xsi:type=\"TrackFileResourceType\">"
+    "<Id>urn:uuid:7d418acb-07a3-4e57-984c-b8ea2f7de4ec</Id>"
+    "<IntrinsicDuration>24</IntrinsicDuration>"
+    "<SourceEncoding>urn:uuid:f00e49a8-0dec-4e6c-95e7-078df988b751</SourceEncoding>"
+    "</Resource>"
+    "</ResourceList>"
+    "</cc:MainImageSequence>"
+    "</SequenceList>"
+    "</Segment>"
+    "</SegmentList>"
+    "</CompositionPlaylist>";
+
 const char *cpl_bad_doc = "<Composition></Composition>";
 
 const char *asset_map_doc =
@@ -366,6 +405,27 @@ static int test_bad_cpl_parsing(FFIMFCPL **cpl)
     return 0;
 }
 
+static int test_bad_resource_cpl_parsing(FFIMFCPL **cpl)
+{
+    xmlDocPtr doc;
+    int ret;
+
+    doc = xmlReadMemory(cpl_bad_resource_doc, strlen(cpl_bad_resource_doc), NULL, NULL, 0);
+    if (doc == NULL) {
+        printf("XML parsing failed.\n");
+        return 1;
+    }
+
+    ret = ff_imf_parse_cpl_from_xml_dom(doc, cpl);
+    xmlFreeDoc(doc);
+    if (ret) {
+        printf("CPL parsing failed.\n");
+        return ret;
+    }
+
+    return 0;
+}
+
 static int check_asset_locator_attributes(IMFAssetLocator *asset, IMFAssetLocator *expected_asset)
 {
 
@@ -533,5 +593,10 @@ int main(int argc, char *argv[])
     }
     printf("#### End failing test ####\n");
 
+    printf("#### The following should emit errors ####\n");
+    if (test_bad_resource_cpl_parsing(&cpl) != 0)
+        ret = 1;
+    printf("#### End emission of errors ####\n");
+
     return ret;
 }
diff --git a/tests/ref/fate/imf b/tests/ref/fate/imf
index 5093167bc7..fdfed8ac17 100644
--- a/tests/ref/fate/imf
+++ b/tests/ref/fate/imf
@@ -53,3 +53,5 @@ For asset: 4:
 #### The following should fail ####
 CPL parsing failed.
 #### End failing test ####
+#### The following should emit errors ####
+#### End emission of errors ####
-- 
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".

Re: [FFmpeg-devel] [PATCH 1/2] avformat/imf: fix invalid resource handling

[Anton Khirnov]

Quoting pal@sandflow.com (2023-04-26 19:53:01)
> From: Pierre-Anthony Lemieux <pal@palemieux.com>
> 
> ---
>  libavformat/imf_cpl.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/libavformat/imf_cpl.c b/libavformat/imf_cpl.c
> index ad84a68b13..a7cf5fa360 100644
> --- a/libavformat/imf_cpl.c
> +++ b/libavformat/imf_cpl.c
> @@ -608,11 +608,10 @@ static int push_main_audio_sequence(xmlNodePtr audio_sequence_elem, FFIMFCPL *cp
>          ret = fill_trackfile_resource(resource_elem,
>                                        &vt->resources[vt->resource_count],
>                                        cpl);
> -        vt->resource_count++;
> -        if (ret) {
> +        if (ret)
>              av_log(NULL, AV_LOG_ERROR, "Invalid Resource\n");
> -            continue;

Should an error here not propagate to the caller?

Unrelated, but all this logging to NULL is very evil.

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