diff mbox series

[FFmpeg-devel,21/21] avformat/matroskaenc: Simplify writing Void elements

Message ID 20200113145134.24086-1-andreas.rheinhardt@gmail.com
State Accepted
Headers show
Series Matroska muxer patches | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt Jan. 13, 2020, 2:51 p.m. UTC
Reserving space in Matroska works by writing a Void element. And until
now this worked as follows: The current position was recorded and the
EBML ID as well as the length field written; then the new position was
recorded to know how much more to write. Afterwards the actual writing
has been performed via ffio_fill().

But it is unnecessary to explicitly use the positions (obtained via
avio_tell()) to find out how much still needs to be written, because the
length of the ID and the length field are known. So rewrite the function
to no longer use them.

Also, given that ffio_fill() uses an int parameter and given that no
current caller (and no sane future caller) will want to reserve several
GB of space, make the size parameter of put_ebml_void() itself an int.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/matroskaenc.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

Comments

Andreas Rheinhardt April 1, 2020, 6:56 p.m. UTC | #1
Andreas Rheinhardt:
> Reserving space in Matroska works by writing a Void element. And until
> now this worked as follows: The current position was recorded and the
> EBML ID as well as the length field written; then the new position was
> recorded to know how much more to write. Afterwards the actual writing
> has been performed via ffio_fill().
> 
> But it is unnecessary to explicitly use the positions (obtained via
> avio_tell()) to find out how much still needs to be written, because the
> length of the ID and the length field are known. So rewrite the function
> to no longer use them.
> 
> Also, given that ffio_fill() uses an int parameter and given that no
> current caller (and no sane future caller) will want to reserve several
> GB of space, make the size parameter of put_ebml_void() itself an int.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/matroskaenc.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 258638e459..e74d59d271 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -288,21 +288,22 @@ static void put_ebml_string(AVIOContext *pb, uint32_t elementid,
>   *
>   * @param size The number of bytes to reserve, which must be at least 2.
>   */
> -static void put_ebml_void(AVIOContext *pb, uint64_t size)
> +static void put_ebml_void(AVIOContext *pb, int size)
>  {
> -    int64_t currentpos = avio_tell(pb);
> -
>      av_assert0(size >= 2);
>  
>      put_ebml_id(pb, EBML_ID_VOID);
>      // we need to subtract the length needed to store the size from the
>      // size we need to reserve so 2 cases, we use 8 bytes to store the
>      // size if possible, 1 byte otherwise
> -    if (size < 10)
> -        put_ebml_num(pb, size - 2, 0);
> -    else
> -        put_ebml_num(pb, size - 9, 8);
> -    ffio_fill(pb, 0, currentpos + size - avio_tell(pb));
> +    if (size < 10) {
> +        size -= 2;
> +        put_ebml_num(pb, size, 0);
> +    } else {
> +        size -= 9;
> +        put_ebml_num(pb, size, 8);
> +    }
> +    ffio_fill(pb, 0, size);
>  }
>  
>  static ebml_master start_ebml_master(AVIOContext *pb, uint32_t elementid,
> 
Any comments on this or the other Matroska muxer patches following this
one? If not, I will probably apply this tomorrow.

- Andreas
diff mbox series

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 258638e459..e74d59d271 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -288,21 +288,22 @@  static void put_ebml_string(AVIOContext *pb, uint32_t elementid,
  *
  * @param size The number of bytes to reserve, which must be at least 2.
  */
-static void put_ebml_void(AVIOContext *pb, uint64_t size)
+static void put_ebml_void(AVIOContext *pb, int size)
 {
-    int64_t currentpos = avio_tell(pb);
-
     av_assert0(size >= 2);
 
     put_ebml_id(pb, EBML_ID_VOID);
     // we need to subtract the length needed to store the size from the
     // size we need to reserve so 2 cases, we use 8 bytes to store the
     // size if possible, 1 byte otherwise
-    if (size < 10)
-        put_ebml_num(pb, size - 2, 0);
-    else
-        put_ebml_num(pb, size - 9, 8);
-    ffio_fill(pb, 0, currentpos + size - avio_tell(pb));
+    if (size < 10) {
+        size -= 2;
+        put_ebml_num(pb, size, 0);
+    } else {
+        size -= 9;
+        put_ebml_num(pb, size, 8);
+    }
+    ffio_fill(pb, 0, size);
 }
 
 static ebml_master start_ebml_master(AVIOContext *pb, uint32_t elementid,