[FFmpeg-devel] [PATCH v2 0/2] avformat/mvdec: make audio stream conditional

[FFmpeg-devel] [PATCH v2 0/2] avformat/mvdec: make audio stream conditional

[John-Paul Stewart]

Changed in v2:
    Allocate the audio stream first to maintain consistent behaviour
    with prior code.

Recent discussion on the list led me to realize that libavformat was
unconditionally creating an audio stream for all SGI movie format
(version 2) files, even when no audio is present in the file.  

A sample of a movie file with no audio can be found at
    http://www.personalprojects.net/ffmpeg/silent.movie

Unpatched ffmpeg will report an audio stream even though no audio is
present.  After the following patch no audio stream is reported.

Incidentally, the silent.movie sample above is at 25fps and can also be
used by anyone who wants to double-check the earlier patch 3c9ffbd009
that reads and sets the framerate.  The sample file is only about 88 KB.


_______________________________________________
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 v2 1/2] avformat/mvdec: make audio stream conditional

[John-Paul Stewart]

Only allocate an audio stream if there is one in the data.  Silicon
Graphics movie format will contain default values (16 bit samples, 2
audio channels, 22050 Hz sample rate) even when no audio is present in
the file.  This confuses FFmpeg into thinking such an audio stream is
present with 0 samples in it.

There is a flag value in the format to indicate whether or not audio is
present.  This patch checks that and behaves accordingly.

Signed-off-by: John-Paul Stewart <jpstewart@personalprojects.net>
---
 libavformat/mvdec.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/libavformat/mvdec.c b/libavformat/mvdec.c
index ea955d2b11..0f0474141b 100644
--- a/libavformat/mvdec.c
+++ b/libavformat/mvdec.c
@@ -45,6 +45,10 @@ typedef struct MvContext {
     int aformat;          ///< audio format
 } MvContext;
 
+/* these magic numbers are defined in moviefile.h on Silicon Grahpics IRIX */
+#define MOVIE_SOUND  1
+#define MOVIE_SILENT 2
+
 #define AUDIO_FORMAT_SIGNED 401
 
 static int mv_probe(const AVProbeData *p)
@@ -305,18 +309,25 @@ static int mv_read_header(AVFormatContext *avctx)
 
         avio_skip(pb, 10);
 
+        fps = av_d2q(av_int2double(avio_rb64(pb)), INT_MAX);
+
         /* allocate audio track first to prevent unnecessary seeking
          * (audio packet always precede video packet for a given frame) */
+        v = avio_rb16(pb);
+        if (v == MOVIE_SOUND) {
+            /* movie has sound so allocate an audio stream */
         ast = avformat_new_stream(avctx, NULL);
         if (!ast)
             return AVERROR(ENOMEM);
+        } else if (v != MOVIE_SILENT)
+            return AVERROR_INVALIDDATA;
+
+        avio_skip(pb, 2);
 
         vst = avformat_new_stream(avctx, NULL);
         if (!vst)
             return AVERROR(ENOMEM);
-        fps = av_d2q(av_int2double(avio_rb64(pb)), INT_MAX);
         avpriv_set_pts_info(vst, 64, fps.den, fps.num);
-        avio_skip(pb, 4);
         vst->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
         vst->avg_frame_rate = fps;
         vst->duration = vst->nb_frames = avio_rb32(pb);
@@ -338,6 +349,7 @@ static int mv_read_header(AVFormatContext *avctx)
         vst->codecpar->height    = avio_rb32(pb);
         avio_skip(pb, 12);
 
+        if (ast) {
         ast->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
         ast->nb_frames          = vst->nb_frames;
         ast->codecpar->sample_rate = avio_rb32(pb);
@@ -373,6 +385,8 @@ static int mv_read_header(AVFormatContext *avctx)
             return AVERROR_INVALIDDATA;
 
         avio_skip(pb, 8);
+        } else
+            avio_skip(pb, 24); /* skip meaningless audio metadata */
 
         var_read_metadata(avctx, "title", 0x80);
         var_read_metadata(avctx, "comment", 0x100);
@@ -386,9 +400,11 @@ static int mv_read_header(AVFormatContext *avctx)
             if (avio_feof(pb))
                 return AVERROR_INVALIDDATA;
             avio_skip(pb, 8);
+            if (ast) {
             av_add_index_entry(ast, pos, timestamp, asize, 0, AVINDEX_KEYFRAME);
-            av_add_index_entry(vst, pos + asize, i, vsize, 0, AVINDEX_KEYFRAME);
             timestamp += asize / (ast->codecpar->channels * (uint64_t)bytes_per_sample);
+            }
+            av_add_index_entry(vst, pos + asize, i, vsize, 0, AVINDEX_KEYFRAME);
         }
     } else if (!version && avio_rb16(pb) == 3) {
         avio_skip(pb, 4);
-- 
2.34.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 v2 2/2] avformat/mvdec: re-indent after last commit

[John-Paul Stewart]

Signed-off-by: John-Paul Stewart <jpstewart@personalprojects.net>
---
 libavformat/mvdec.c | 72 ++++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/libavformat/mvdec.c b/libavformat/mvdec.c
index 0f0474141b..4f233aff5f 100644
--- a/libavformat/mvdec.c
+++ b/libavformat/mvdec.c
@@ -316,9 +316,9 @@ static int mv_read_header(AVFormatContext *avctx)
         v = avio_rb16(pb);
         if (v == MOVIE_SOUND) {
             /* movie has sound so allocate an audio stream */
-        ast = avformat_new_stream(avctx, NULL);
-        if (!ast)
-            return AVERROR(ENOMEM);
+            ast = avformat_new_stream(avctx, NULL);
+            if (!ast)
+                return AVERROR(ENOMEM);
         } else if (v != MOVIE_SILENT)
             return AVERROR_INVALIDDATA;
 
@@ -350,41 +350,41 @@ static int mv_read_header(AVFormatContext *avctx)
         avio_skip(pb, 12);
 
         if (ast) {
-        ast->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
-        ast->nb_frames          = vst->nb_frames;
-        ast->codecpar->sample_rate = avio_rb32(pb);
-        if (ast->codecpar->sample_rate <= 0) {
-            av_log(avctx, AV_LOG_ERROR, "Invalid sample rate %d\n", ast->codecpar->sample_rate);
-            return AVERROR_INVALIDDATA;
-        }
-        avpriv_set_pts_info(ast, 33, 1, ast->codecpar->sample_rate);
-
-        bytes_per_sample = avio_rb32(pb);
-
-        v = avio_rb32(pb);
-        if (v == AUDIO_FORMAT_SIGNED) {
-            switch (bytes_per_sample) {
-            case 1:
-                ast->codecpar->codec_id = AV_CODEC_ID_PCM_S8;
-                break;
-            case 2:
-                ast->codecpar->codec_id = AV_CODEC_ID_PCM_S16BE;
-                break;
-            default:
-                avpriv_request_sample(avctx, "Audio sample size %i bytes", bytes_per_sample);
-                break;
+            ast->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
+            ast->nb_frames = vst->nb_frames;
+            ast->codecpar->sample_rate = avio_rb32(pb);
+            if (ast->codecpar->sample_rate <= 0) {
+                av_log(avctx, AV_LOG_ERROR, "Invalid sample rate %d\n", ast->codecpar->sample_rate);
+                return AVERROR_INVALIDDATA;
+            }
+            avpriv_set_pts_info(ast, 33, 1, ast->codecpar->sample_rate);
+
+            bytes_per_sample = avio_rb32(pb);
+
+            v = avio_rb32(pb);
+            if (v == AUDIO_FORMAT_SIGNED) {
+                switch (bytes_per_sample) {
+                case 1:
+                    ast->codecpar->codec_id = AV_CODEC_ID_PCM_S8;
+                    break;
+                case 2:
+                    ast->codecpar->codec_id = AV_CODEC_ID_PCM_S16BE;
+                    break;
+                default:
+                    avpriv_request_sample(avctx, "Audio sample size %i bytes", bytes_per_sample);
+                    break;
+                }
+            } else {
+                avpriv_request_sample(avctx, "Audio compression (format %i)", v);
             }
-        } else {
-            avpriv_request_sample(avctx, "Audio compression (format %i)", v);
-        }
 
-        if (bytes_per_sample == 0)
-            return AVERROR_INVALIDDATA;
+            if (bytes_per_sample == 0)
+                return AVERROR_INVALIDDATA;
 
-        if (set_channels(avctx, ast, avio_rb32(pb)) < 0)
-            return AVERROR_INVALIDDATA;
+            if (set_channels(avctx, ast, avio_rb32(pb)) < 0)
+                return AVERROR_INVALIDDATA;
 
-        avio_skip(pb, 8);
+            avio_skip(pb, 8);
         } else
             avio_skip(pb, 24); /* skip meaningless audio metadata */
 
@@ -401,8 +401,8 @@ static int mv_read_header(AVFormatContext *avctx)
                 return AVERROR_INVALIDDATA;
             avio_skip(pb, 8);
             if (ast) {
-            av_add_index_entry(ast, pos, timestamp, asize, 0, AVINDEX_KEYFRAME);
-            timestamp += asize / (ast->codecpar->channels * (uint64_t)bytes_per_sample);
+                av_add_index_entry(ast, pos, timestamp, asize, 0, AVINDEX_KEYFRAME);
+                timestamp += asize / (ast->codecpar->channels * (uint64_t)bytes_per_sample);
             }
             av_add_index_entry(vst, pos + asize, i, vsize, 0, AVINDEX_KEYFRAME);
         }
-- 
2.34.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 v2 0/2] avformat/mvdec: make audio stream conditional

[Peter Ross]

[-- Attachment #1.1: Type: text/plain, Size: 1027 bytes --]

On Fri, Dec 31, 2021 at 08:11:46PM -0500, John-Paul Stewart wrote:
> Changed in v2:
>     Allocate the audio stream first to maintain consistent behaviour
>     with prior code.
> 
> Recent discussion on the list led me to realize that libavformat was
> unconditionally creating an audio stream for all SGI movie format
> (version 2) files, even when no audio is present in the file.  
> 
> A sample of a movie file with no audio can be found at
>     http://www.personalprojects.net/ffmpeg/silent.movie
> 
> Unpatched ffmpeg will report an audio stream even though no audio is
> present.  After the following patch no audio stream is reported.
> 
> Incidentally, the silent.movie sample above is at 25fps and can also be
> used by anyone who wants to double-check the earlier patch 3c9ffbd009
> that reads and sets the framerate.  The sample file is only about 88 KB.

v2 patchset looks good to me. I will apply in a couple of days. Thanks.

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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