diff mbox

[FFmpeg-devel] avformat/matroskaenc: add reserve free space option

Message ID CAPgnUfDSd-n5jxjX0GoMry73jA9qYcVxfD2L8KwsJY2xPZwbew@mail.gmail.com
State Superseded
Headers show

Commit Message

Sigríður Regína Sigurþórsdóttir Sept. 6, 2018, 5:39 p.m. UTC 
On Wed, Sep 5, 2018 at 6:29 PM James Darnley <james.darnley@gmail.com> wrote:
>
> On 2018-09-05 22:52, Sigríður Regína Sigurþórsdóttir wrote:
> > +    {"reserve_free_space", "Reserve a given amount of space at the
> > beginning og the file for unspecified purpose."
>
> I added the "metadata_header_padding" global option many years ago.  Can
> you not reuse it for this purpose?  Is it not likely to be "metadata"
> that another software might fill this with?

Thank you for the suggestion. Here is an updated version using the
"metadata_header_padding".


---
 libavformat/matroskaenc.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

James Darnley Sept. 6, 2018, 7:18 p.m. UTC | #1 
On 2018-09-06 19:39, Sigríður Regína Sigurþórsdóttir wrote:
> +    if (s->metadata_header_padding) {
> +        if (s->metadata_header_padding == 1)
> +            s->metadata_header_padding++;
> +        put_ebml_void(pb, s->metadata_header_padding);
> +    }

Unfortunately I was forced to make the default -1 so you want to check
that the value is greater than 0 rather than just true.

Furthermore I think you will still want to add to Changelog making a
note that the matroska muxer will now listen to metadata_header_padding.
 That may also want a micro version bump so that library users can check.
Carl Eugen Hoyos Sept. 6, 2018, 7:23 p.m. UTC | #2 
2018-09-06 21:18 GMT+02:00, James Darnley <james.darnley@gmail.com>:
> On 2018-09-06 19:39, Sigríður Regína Sigurþórsdóttir wrote:
>> +    if (s->metadata_header_padding) {
>> +        if (s->metadata_header_padding == 1)
>> +            s->metadata_header_padding++;
>> +        put_ebml_void(pb, s->metadata_header_padding);
>> +    }
>
> Unfortunately I was forced to make the default -1 so you want to check
> that the value is greater than 0 rather than just true.
>
> Furthermore I think you will still want to add to Changelog making a
> note that the matroska muxer will now listen to metadata_header_padding.

I don't remember a new option that warranted a Changelog entry...

>  That may also want a micro version bump so that library users can check.

Of course!

Carl Eugen
James Almer Sept. 6, 2018, 7:31 p.m. UTC | #3 
On 9/6/2018 4:18 PM, James Darnley wrote:
> On 2018-09-06 19:39, Sigríður Regína Sigurþórsdóttir wrote:
>> +    if (s->metadata_header_padding) {
>> +        if (s->metadata_header_padding == 1)
>> +            s->metadata_header_padding++;
>> +        put_ebml_void(pb, s->metadata_header_padding);
>> +    }
> 
> Unfortunately I was forced to make the default -1 so you want to check
> that the value is greater than 0 rather than just true.
> 
> Furthermore I think you will still want to add to Changelog making a
> note that the matroska muxer will now listen to metadata_header_padding.

No, this kind of change doesn't justify a Changelog entry as mentioned
before.

>  That may also want a micro version bump so that library users can check.

Micro version bump is ok.

> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
diff mbox

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 09a62e1..3d8ec3c 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -2005,6 +2005,11 @@  static int mkv_write_header(AVFormatContext *s)
         ret = AVERROR(ENOMEM);
         goto fail;
     }
+    if (s->metadata_header_padding) {
+        if (s->metadata_header_padding == 1)
+            s->metadata_header_padding++;
+        put_ebml_void(pb, s->metadata_header_padding);
+    }
     if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && mkv->reserve_cues_space) {
         mkv->cues_pos = avio_tell(pb);
         if (mkv->reserve_cues_space == 1)