diff mbox series

[FFmpeg-devel] cafenc: fill in avg. packet size later if unknown

Message ID 20210710011006.3383868-1-roman.beranek@prusa3d.cz
State Accepted
Commit ed240db9e3122d4dccbd1bee7d12cd50a0edc807
Headers show
Series [FFmpeg-devel] cafenc: fill in avg. packet size later if unknown | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Roman Beranek July 10, 2021, 1:10 a.m. UTC 
Frame size of Opus stream was previously presumed here to be 960 samples
(20ms), however sizes of 120, 240, 480, 1920, and 2880 are also allowed.
It can also alter on a per-packet basis and even multiple frames may be
present in a single packet according to the specification, for the sake
of simplicity however, let us assume that this doesn't occur.

Because the mFramesPerPacket field, representing the number of samples
per packet in the ffmpeg terminilogy, is the key factor in calculating
packet durations and all that follows from that (index, bitrate, ...),
it is crucial to get right.

Therefore, if the packet size is not available ahead of time (as it is in
the case of Opus), calculate an average from the stream duration once we
know how many packets there are and update the filed in the header.
---
 libavformat/cafenc.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Comments

ffmpegandmahanstreamer@lolcow.email July 11, 2021, 12:33 p.m. UTC | #1 
On 2021-07-10 03:42, Lynne wrote:
> This doesn't move the pointer back to the file end if par->block_align 
> is set.
> I think that's fine though, since the function writes the trailer, 
> which should
> mean that nothing more needs to be written.
> Patch LGTM. But please, someone yell at Apple to support Opus in MP4,
> WebM and OGG, as terrible as that is.

Doesn't apple already support webm and opus?
> _______________________________________________
> 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".
Lynne July 12, 2021, 7:58 a.m. UTC | #2 
10 Jul 2021, 09:42 by dev@lynne.ee:

> 10 Jul 2021, 03:10 by roman.beranek@prusa3d.cz:
>
>> Frame size of Opus stream was previously presumed here to be 960 samples
>> (20ms), however sizes of 120, 240, 480, 1920, and 2880 are also allowed.
>> It can also alter on a per-packet basis and even multiple frames may be
>> present in a single packet according to the specification, for the sake
>> of simplicity however, let us assume that this doesn't occur.
>>
>
> Actually 120ms frames are the maximum, so 5760 samples, but that's
> irrelevant to the patch.
>  
>
>> if (pb->seekable & AVIO_SEEKABLE_NORMAL) {
>>  int64_t file_size = avio_tell(pb);
>>  
>>  avio_seek(pb, caf->data, SEEK_SET);
>>  avio_wb64(pb, file_size - caf->data - 8);
>> -        avio_seek(pb, file_size, SEEK_SET);
>>  if (!par->block_align) {
>> +            int packet_size = samples_per_packet(par->codec_id, par->channels, par->block_align);
>> +            if (!packet_size) {
>> +                packet_size = st->duration / (caf->packets - 1);
>> +                avio_seek(pb, FRAME_SIZE_OFFSET, SEEK_SET);
>> +                avio_wb32(pb, packet_size);
>> +            }
>> +            avio_seek(pb, file_size, SEEK_SET);
>>  ffio_wfourcc(pb, "pakt");
>>  avio_wb64(pb, caf->size_entries_used + 24);
>>  avio_wb64(pb, caf->packets); ///< mNumberPackets
>> -            avio_wb64(pb, caf->packets * samples_per_packet(par->codec_id, par->channels, par->block_align)); ///< mNumberValidFrames
>> +            avio_wb64(pb, caf->packets * packet_size); ///< mNumberValidFrames
>>  avio_wb32(pb, 0); ///< mPrimingFrames
>>  avio_wb32(pb, 0); ///< mRemainderFrames
>>  avio_write(pb, caf->pkt_sizes, caf->size_entries_used);
>>
>
> This doesn't move the pointer back to the file end if par->block_align is set.
> I think that's fine though, since the function writes the trailer, which should
> mean that nothing more needs to be written.
> Patch LGTM. But please, someone yell at Apple to support Opus in MP4,
> WebM and OGG, as terrible as that is.
>

Patch pushed, thanks
Roman Beranek July 12, 2021, 9:42 a.m. UTC | #3 
> This doesn't move the pointer back to the file end if par->block_align is set.
> I think that's fine though, since the function writes the trailer, which should
> mean that nothing more needs to be written.

Is it a convention to leave the pointer positioned at the end of
the file?
diff mbox series

Patch

diff --git a/libavformat/cafenc.c b/libavformat/cafenc.c
index 422beaf93d..816e978945 100644
--- a/libavformat/cafenc.c
+++ b/libavformat/cafenc.c
@@ -26,6 +26,8 @@ 
 #include "libavutil/intfloat.h"
 #include "libavutil/dict.h"
 
+#define FRAME_SIZE_OFFSET 40
+
 typedef struct {
     int64_t data;
     uint8_t *pkt_sizes;
@@ -81,8 +83,6 @@  static uint32_t samples_per_packet(enum AVCodecID codec_id, int channels, int bl
         return 320;
     case AV_CODEC_ID_MP1:
         return 384;
-    case AV_CODEC_ID_OPUS:
-        return 960;
     case AV_CODEC_ID_MP2:
     case AV_CODEC_ID_MP3:
         return 1152;
@@ -240,19 +240,26 @@  static int caf_write_trailer(AVFormatContext *s)
 {
     CAFContext *caf = s->priv_data;
     AVIOContext *pb = s->pb;
-    AVCodecParameters *par = s->streams[0]->codecpar;
+    AVStream *st = s->streams[0];
+    AVCodecParameters *par = st->codecpar;
 
     if (pb->seekable & AVIO_SEEKABLE_NORMAL) {
         int64_t file_size = avio_tell(pb);
 
         avio_seek(pb, caf->data, SEEK_SET);
         avio_wb64(pb, file_size - caf->data - 8);
-        avio_seek(pb, file_size, SEEK_SET);
         if (!par->block_align) {
+            int packet_size = samples_per_packet(par->codec_id, par->channels, par->block_align);
+            if (!packet_size) {
+                packet_size = st->duration / (caf->packets - 1);
+                avio_seek(pb, FRAME_SIZE_OFFSET, SEEK_SET);
+                avio_wb32(pb, packet_size);
+            }
+            avio_seek(pb, file_size, SEEK_SET);
             ffio_wfourcc(pb, "pakt");
             avio_wb64(pb, caf->size_entries_used + 24);
             avio_wb64(pb, caf->packets); ///< mNumberPackets
-            avio_wb64(pb, caf->packets * samples_per_packet(par->codec_id, par->channels, par->block_align)); ///< mNumberValidFrames
+            avio_wb64(pb, caf->packets * packet_size); ///< mNumberValidFrames
             avio_wb32(pb, 0); ///< mPrimingFrames
             avio_wb32(pb, 0); ///< mRemainderFrames
             avio_write(pb, caf->pkt_sizes, caf->size_entries_used);