[FFmpeg-devel] [PATCH 04/20] avformat/matroskaenc: Reuse random seed

[Andreas Rheinhardt]

Steve Lhomme:
>> On April 19, 2020 9:11 AM Andreas Rheinhardt <andreas.rheinhardt at gmail.com> wrote:
>>
>>  
>> Steve Lhomme:
>>> Wouldn't it be better to set the AVLFG in the MatroskaMuxContext in mkv_init and keep using it when needed ?
>>>
>>> Then you can still update UIDs in the location you really need to create them.
>>>
>> What other UIDs do I need? UIDs for tracks and attachments are created
>> in init as is the SegmentUID. The EditionUID is not written as we only
>> support ordinary chapters for now (FFmpeg does not have nested chapters
>> or multiple editions or hierarchical metadata -- I really don't know how
>> this could be supported). And the ChapterUIDs are not created randomly.
> 
> That's a general rule so you don't need to adapt everytime you need a new UIDs. And it's easier/cleaner to generate UIDs when the item they relate to is created.
> 
> Anyway, it's not a blocker, just a general remark.
> 
Several of these patches (including this one) have already been applied;
the Cues patch has not (the reason for this is that I wanted to wait for
this until I have added a test for DASH audio just in order to show that
this patch does not affect the Cues for DASH audio at all).


> As for other UIDs they should be randomized (hence the Unique in UIDs) unless you're remuxing or taking the values from other sources which is sometimes needed for advanced chapter handling or referencing a part of the same mux via a UID.

I know. I will implement preserving the TrackUID during remuxing
(streamcopying) some day. But advanced chapter handling is very far off
given the fundamental differences of FFmpeg's chapter handling and
Matroska's. (The current handling of Tags leaves much to be desired, too.)

- Andreas

>  
>> - Andreas
>>
>>>> On April 5, 2020 5:59 PM Andreas Rheinhardt <andreas.rheinhardt at gmail.com> wrote:
>>>>
>>>>  
>>>> This commit reuses the random seed generated in mkv_init() (to determine
>>>> the TrackUIDs) for the SegmentUID in order to avoid a potentially
>>>> expensive call to av_get_random_seed().
>>>>
>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>>> ---
>>>>  libavformat/matroskaenc.c | 19 +++++++++----------
>>>>  1 file changed, 9 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>>>> index d2046193ee..b93c50cb01 100644
>>>> --- a/libavformat/matroskaenc.c
>>>> +++ b/libavformat/matroskaenc.c
>>>> @@ -161,6 +161,8 @@ typedef struct MatroskaMuxContext {
>>>>      int wrote_chapters;
>>>>  
>>>>      int allow_raw_vfw;
>>>> +
>>>> +    uint32_t segment_uid[4];
>>>>  } MatroskaMuxContext;
>>>>  
>>>>  /** 2 bytes * 7 for EBML IDs, 7 1-byte EBML lengths, 6 1-byte uint,
>>>> @@ -1821,15 +1823,7 @@ static int mkv_write_header(AVFormatContext *s)
>>>>              put_ebml_string(pb, MATROSKA_ID_WRITINGAPP, LIBAVFORMAT_IDENT);
>>>>  
>>>>          if (mkv->mode != MODE_WEBM) {
>>>> -            uint32_t segment_uid[4];
>>>> -            AVLFG lfg;
>>>> -
>>>> -            av_lfg_init(&lfg, av_get_random_seed());
>>>> -
>>>> -            for (i = 0; i < 4; i++)
>>>> -                segment_uid[i] = av_lfg_get(&lfg);
>>>> -
>>>> -            put_ebml_binary(pb, MATROSKA_ID_SEGMENTUID, segment_uid, 16);
>>>> +            put_ebml_binary(pb, MATROSKA_ID_SEGMENTUID, mkv->segment_uid, 16);
>>>>          }
>>>>      } else {
>>>>          const char *ident = "Lavf";
>>>> @@ -2661,9 +2655,14 @@ static int mkv_init(struct AVFormatContext *s)
>>>>          return AVERROR(ENOMEM);
>>>>      }
>>>>  
>>>> -    if (!(s->flags & AVFMT_FLAG_BITEXACT))
>>>> +    if (!(s->flags & AVFMT_FLAG_BITEXACT)) {
>>>>          av_lfg_init(&c, av_get_random_seed());
>>>>  
>>>> +        // Calculate the SegmentUID now in order not to waste our random seed.
>>>> +        for (i = 0; i < 4; i++)
>>>> +            mkv->segment_uid[i] = av_lfg_get(&c);
>>>> +    }
>>>> +
>>>>      for (i = 0; i < s->nb_streams; i++) {
>>>>          mkv_track *track = &mkv->tracks[i];
>>>>  
>>>> -- 
>>>> 2.20.1
>>>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>