diff mbox series

[FFmpeg-devel] avformat/movenc: move the AVProducerReferenceTime struct to MOVTrack

Message ID 20200324235258.610-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel] avformat/movenc: move the AVProducerReferenceTime struct to MOVTrack
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

James Almer March 24, 2020, 11:52 p.m. UTC
We only care about the prft timestamp of the first sample in each chunk, so
no need to store every single one within each MOVIentry.

Should reduce memory usage.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/movenc.c | 23 +++++++++++++----------
 libavformat/movenc.h |  3 ++-
 2 files changed, 15 insertions(+), 11 deletions(-)

Comments

Andreas Rheinhardt March 25, 2020, 12:01 a.m. UTC | #1
James Almer:
> We only care about the prft timestamp of the first sample in each chunk, so
> no need to store every single one within each MOVIentry.
> 
> Should reduce memory usage.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavformat/movenc.c | 23 +++++++++++++----------
>  libavformat/movenc.h |  3 ++-
>  2 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 85df5d1374..b748b92314 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -4675,11 +4675,11 @@ static int mov_write_prft_tag(AVIOContext *pb, MOVMuxContext *mov, int tracks)
>      }
>  
>      if (mov->write_prft == MOV_PRFT_SRC_WALLCLOCK) {
> -        if (first_track->cluster[0].prft.wallclock) {
> +        if (first_track->prft.wallclock) {
>              /* Round the NTP time to whole milliseconds. */
> -            ntp_ts = ff_get_formatted_ntp_time((first_track->cluster[0].prft.wallclock / 1000) * 1000 +
> +            ntp_ts = ff_get_formatted_ntp_time((first_track->prft.wallclock / 1000) * 1000 +
>                                                 NTP_OFFSET_US);
> -            flags = first_track->cluster[0].prft.flags;
> +            flags = first_track->prft.flags;
>          } else
>              ntp_ts = ff_get_formatted_ntp_time(ff_ntp_time());
>      } else if (mov->write_prft == MOV_PRFT_SRC_PTS) {
> @@ -5391,10 +5391,8 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
>      AVIOContext *pb = s->pb;
>      MOVTrack *trk = &mov->tracks[pkt->stream_index];
>      AVCodecParameters *par = trk->par;
> -    AVProducerReferenceTime *prft;
>      unsigned int samples_in_chunk = 0;
>      int size = pkt->size, ret = 0, offset = 0;
> -    int prft_size;
>      uint8_t *reformatted_data = NULL;
>  
>      ret = check_pkt(s, pkt);
> @@ -5675,11 +5673,16 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
>          trk->has_disposable++;
>      }
>  
> -    prft = (AVProducerReferenceTime *)av_packet_get_side_data(pkt, AV_PKT_DATA_PRFT, &prft_size);
> -    if (prft && prft_size == sizeof(AVProducerReferenceTime))
> -        memcpy(&trk->cluster[trk->entry].prft, prft, prft_size);
> -    else
> -        memset(&trk->cluster[trk->entry].prft, 0, sizeof(AVProducerReferenceTime));
> +    if (!trk->entry) {
> +        /* We only want the prft for the first sample in a chunk */
> +        int prft_size;
> +        AVProducerReferenceTime *prft =
> +            (AVProducerReferenceTime *)av_packet_get_side_data(pkt, AV_PKT_DATA_PRFT, &prft_size);
> +        if (prft && prft_size == sizeof(trk->prft))
> +            memcpy(&trk->prft, prft, prft_size);
> +        else
> +            memset(&trk->prft, 0, sizeof(trk->prft));
> +    }
>  
>      trk->entry++;
>      trk->sample_count += samples_in_chunk;
> diff --git a/libavformat/movenc.h b/libavformat/movenc.h
> index 997b2d61c0..d394bf757f 100644
> --- a/libavformat/movenc.h
> +++ b/libavformat/movenc.h
> @@ -56,7 +56,6 @@ typedef struct MOVIentry {
>  #define MOV_PARTIAL_SYNC_SAMPLE 0x0002
>  #define MOV_DISPOSABLE_SAMPLE   0x0004
>      uint32_t     flags;
> -    AVProducerReferenceTime prft;
>  } MOVIentry;

5717cd80dcb825efcbcb0b936a02b755cd2d5f62 added the pts field to
MOVIentry for writing the prtf box and it seems that only the very first
pts are ever needed (just like for the AVProducerReferenceTime). So can
this field be moved to MOVTrack, too?

- Andreas

>  
>  typedef struct HintSample {
> @@ -160,6 +159,8 @@ typedef struct MOVTrack {
>  
>      MOVMuxCencContext cenc;
>  
> +    AVProducerReferenceTime prft;
> +
>      uint32_t palette[AVPALETTE_COUNT];
>      int pal_done;
>  
>
James Almer March 25, 2020, 12:12 a.m. UTC | #2
On 3/24/2020 9:01 PM, Andreas Rheinhardt wrote:
> James Almer:
>> We only care about the prft timestamp of the first sample in each chunk, so
>> no need to store every single one within each MOVIentry.
>>
>> Should reduce memory usage.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  libavformat/movenc.c | 23 +++++++++++++----------
>>  libavformat/movenc.h |  3 ++-
>>  2 files changed, 15 insertions(+), 11 deletions(-)
>>
>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> index 85df5d1374..b748b92314 100644
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -4675,11 +4675,11 @@ static int mov_write_prft_tag(AVIOContext *pb, MOVMuxContext *mov, int tracks)
>>      }
>>  
>>      if (mov->write_prft == MOV_PRFT_SRC_WALLCLOCK) {
>> -        if (first_track->cluster[0].prft.wallclock) {
>> +        if (first_track->prft.wallclock) {
>>              /* Round the NTP time to whole milliseconds. */
>> -            ntp_ts = ff_get_formatted_ntp_time((first_track->cluster[0].prft.wallclock / 1000) * 1000 +
>> +            ntp_ts = ff_get_formatted_ntp_time((first_track->prft.wallclock / 1000) * 1000 +
>>                                                 NTP_OFFSET_US);
>> -            flags = first_track->cluster[0].prft.flags;
>> +            flags = first_track->prft.flags;
>>          } else
>>              ntp_ts = ff_get_formatted_ntp_time(ff_ntp_time());
>>      } else if (mov->write_prft == MOV_PRFT_SRC_PTS) {
>> @@ -5391,10 +5391,8 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
>>      AVIOContext *pb = s->pb;
>>      MOVTrack *trk = &mov->tracks[pkt->stream_index];
>>      AVCodecParameters *par = trk->par;
>> -    AVProducerReferenceTime *prft;
>>      unsigned int samples_in_chunk = 0;
>>      int size = pkt->size, ret = 0, offset = 0;
>> -    int prft_size;
>>      uint8_t *reformatted_data = NULL;
>>  
>>      ret = check_pkt(s, pkt);
>> @@ -5675,11 +5673,16 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
>>          trk->has_disposable++;
>>      }
>>  
>> -    prft = (AVProducerReferenceTime *)av_packet_get_side_data(pkt, AV_PKT_DATA_PRFT, &prft_size);
>> -    if (prft && prft_size == sizeof(AVProducerReferenceTime))
>> -        memcpy(&trk->cluster[trk->entry].prft, prft, prft_size);
>> -    else
>> -        memset(&trk->cluster[trk->entry].prft, 0, sizeof(AVProducerReferenceTime));
>> +    if (!trk->entry) {
>> +        /* We only want the prft for the first sample in a chunk */
>> +        int prft_size;
>> +        AVProducerReferenceTime *prft =
>> +            (AVProducerReferenceTime *)av_packet_get_side_data(pkt, AV_PKT_DATA_PRFT, &prft_size);
>> +        if (prft && prft_size == sizeof(trk->prft))
>> +            memcpy(&trk->prft, prft, prft_size);
>> +        else
>> +            memset(&trk->prft, 0, sizeof(trk->prft));
>> +    }
>>  
>>      trk->entry++;
>>      trk->sample_count += samples_in_chunk;
>> diff --git a/libavformat/movenc.h b/libavformat/movenc.h
>> index 997b2d61c0..d394bf757f 100644
>> --- a/libavformat/movenc.h
>> +++ b/libavformat/movenc.h
>> @@ -56,7 +56,6 @@ typedef struct MOVIentry {
>>  #define MOV_PARTIAL_SYNC_SAMPLE 0x0002
>>  #define MOV_DISPOSABLE_SAMPLE   0x0004
>>      uint32_t     flags;
>> -    AVProducerReferenceTime prft;
>>  } MOVIentry;
> 
> 5717cd80dcb825efcbcb0b936a02b755cd2d5f62 added the pts field to
> MOVIentry for writing the prtf box and it seems that only the very first
> pts are ever needed (just like for the AVProducerReferenceTime). So can
> this field be moved to MOVTrack, too?

It could, but i'd rather not. It will look weird in MOVTrack (like prft
does after this patch, for that matter), and as is it could be used
later for some other purpose.

> 
> - Andreas
> 
>>  
>>  typedef struct HintSample {
>> @@ -160,6 +159,8 @@ typedef struct MOVTrack {
>>  
>>      MOVMuxCencContext cenc;
>>  
>> +    AVProducerReferenceTime prft;
>> +
>>      uint32_t palette[AVPALETTE_COUNT];
>>      int pal_done;
>>  
>>
> 
> _______________________________________________
> 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".
>
diff mbox series

Patch

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 85df5d1374..b748b92314 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -4675,11 +4675,11 @@  static int mov_write_prft_tag(AVIOContext *pb, MOVMuxContext *mov, int tracks)
     }
 
     if (mov->write_prft == MOV_PRFT_SRC_WALLCLOCK) {
-        if (first_track->cluster[0].prft.wallclock) {
+        if (first_track->prft.wallclock) {
             /* Round the NTP time to whole milliseconds. */
-            ntp_ts = ff_get_formatted_ntp_time((first_track->cluster[0].prft.wallclock / 1000) * 1000 +
+            ntp_ts = ff_get_formatted_ntp_time((first_track->prft.wallclock / 1000) * 1000 +
                                                NTP_OFFSET_US);
-            flags = first_track->cluster[0].prft.flags;
+            flags = first_track->prft.flags;
         } else
             ntp_ts = ff_get_formatted_ntp_time(ff_ntp_time());
     } else if (mov->write_prft == MOV_PRFT_SRC_PTS) {
@@ -5391,10 +5391,8 @@  int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
     AVIOContext *pb = s->pb;
     MOVTrack *trk = &mov->tracks[pkt->stream_index];
     AVCodecParameters *par = trk->par;
-    AVProducerReferenceTime *prft;
     unsigned int samples_in_chunk = 0;
     int size = pkt->size, ret = 0, offset = 0;
-    int prft_size;
     uint8_t *reformatted_data = NULL;
 
     ret = check_pkt(s, pkt);
@@ -5675,11 +5673,16 @@  int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
         trk->has_disposable++;
     }
 
-    prft = (AVProducerReferenceTime *)av_packet_get_side_data(pkt, AV_PKT_DATA_PRFT, &prft_size);
-    if (prft && prft_size == sizeof(AVProducerReferenceTime))
-        memcpy(&trk->cluster[trk->entry].prft, prft, prft_size);
-    else
-        memset(&trk->cluster[trk->entry].prft, 0, sizeof(AVProducerReferenceTime));
+    if (!trk->entry) {
+        /* We only want the prft for the first sample in a chunk */
+        int prft_size;
+        AVProducerReferenceTime *prft =
+            (AVProducerReferenceTime *)av_packet_get_side_data(pkt, AV_PKT_DATA_PRFT, &prft_size);
+        if (prft && prft_size == sizeof(trk->prft))
+            memcpy(&trk->prft, prft, prft_size);
+        else
+            memset(&trk->prft, 0, sizeof(trk->prft));
+    }
 
     trk->entry++;
     trk->sample_count += samples_in_chunk;
diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index 997b2d61c0..d394bf757f 100644
--- a/libavformat/movenc.h
+++ b/libavformat/movenc.h
@@ -56,7 +56,6 @@  typedef struct MOVIentry {
 #define MOV_PARTIAL_SYNC_SAMPLE 0x0002
 #define MOV_DISPOSABLE_SAMPLE   0x0004
     uint32_t     flags;
-    AVProducerReferenceTime prft;
 } MOVIentry;
 
 typedef struct HintSample {
@@ -160,6 +159,8 @@  typedef struct MOVTrack {
 
     MOVMuxCencContext cenc;
 
+    AVProducerReferenceTime prft;
+
     uint32_t palette[AVPALETTE_COUNT];
     int pal_done;