movdec: Set frame_size for AMR

Message ID 1320490713-13860-1-git-send-email-martin@martin.st
State Committed
Commit 237f13290bcc1c69cf103dede7f546a2a0706b5c
Headers show

Commit Message

Martin Storsjö Nov. 5, 2011, 10:58 a.m.
From: Carl Eugen Hoyos <cehoyos@ag.or.at>

Earlier, sc->samples_per_frame was used for setting the frame size,
but all files don't have that set properly. The frame size is a
known constant for these codecs.

If frame_size isn't set, the mov/3gp muxer refuses to mux it.

This fixes stream copy of audio from
https://roundup.libav.org/file1248/Video_With_AMR-NB_Audio.3gp
to another 3gp file (roundup issue 2468).
---

Since the previous version, I split the both AMR cases into
separate switch cases, since the only common part now is
setting channels = 1.

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

Comments

Justin Ruggles Nov. 5, 2011, 1:26 p.m. | #1
On 11/05/2011 06:58 AM, Martin Storsjö wrote:

> From: Carl Eugen Hoyos <cehoyos@ag.or.at>
> 
> Earlier, sc->samples_per_frame was used for setting the frame size,
> but all files don't have that set properly. The frame size is a
> known constant for these codecs.
> 
> If frame_size isn't set, the mov/3gp muxer refuses to mux it.
> 
> This fixes stream copy of audio from
> https://roundup.libav.org/file1248/Video_With_AMR-NB_Audio.3gp
> to another 3gp file (roundup issue 2468).


That frame_size check in the mov/3gp muxer that prevents stream copy in
this case is unnecessary. The field is not even used when writing 3gp.
Even if we remove all setting of frame_size at all in the demuxer, the
timestamps are still able to be calculated correctly, and the muxer will
write correct output if the frame_size check is removed. For mov muxing,
where frame_size is used in the SoundDescription, the correct packet
duration is written in the stts atom even without frame_size set.

That said, I don't object to the patch, as it doesn't do any harm.

But separate from this, I think the mov muxer needs to be fixed... it
should not require frame_size to be set, at least not for all cases.

-Justin
Martin Storsjö Nov. 5, 2011, 2:32 p.m. | #2
On Sat, 5 Nov 2011, Justin Ruggles wrote:

> That frame_size check in the mov/3gp muxer that prevents stream copy in
> this case is unnecessary. The field is not even used when writing 3gp.
> Even if we remove all setting of frame_size at all in the demuxer, the
> timestamps are still able to be calculated correctly, and the muxer will
> write correct output if the frame_size check is removed. For mov muxing,
> where frame_size is used in the SoundDescription, the correct packet
> duration is written in the stts atom even without frame_size set.
>
> That said, I don't object to the patch, as it doesn't do any harm.

Ok, applied.

> But separate from this, I think the mov muxer needs to be fixed... it
> should not require frame_size to be set, at least not for all cases.

Yes, I guess that would be good.

// Martin

Patch

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 2036f51..351058c 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1291,14 +1291,16 @@  int ff_mov_read_stsd_entries(MOVContext *c, AVIOContext *pb, int entries)
         st->codec->channels= 1; /* really needed */
         break;
     case CODEC_ID_AMR_NB:
-    case CODEC_ID_AMR_WB:
-        st->codec->frame_size= sc->samples_per_frame;
         st->codec->channels= 1; /* really needed */
         /* force sample rate for amr, stsd in 3gp does not store sample rate */
-        if (st->codec->codec_id == CODEC_ID_AMR_NB)
-            st->codec->sample_rate = 8000;
-        else if (st->codec->codec_id == CODEC_ID_AMR_WB)
-            st->codec->sample_rate = 16000;
+        st->codec->sample_rate = 8000;
+        /* force frame_size, too, samples_per_frame isn't always set properly */
+        st->codec->frame_size  = 160;
+        break;
+    case CODEC_ID_AMR_WB:
+        st->codec->channels    = 1;
+        st->codec->sample_rate = 16000;
+        st->codec->frame_size  = 320;
         break;
     case CODEC_ID_MP2:
     case CODEC_ID_MP3: