diff mbox series

[FFmpeg-devel,3/6] avformat/argo_asf: use ArgoASFContext in muxer

Message ID 20200808075914.2296555-3-zane@zanevaniperen.com
State Superseded
Headers show
Series [FFmpeg-devel,1/6] avformat/argo_asf: don't check or probe file version
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Zane van Iperen Aug. 8, 2020, 8 a.m. UTC
Preparation for options.

Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
---
 libavformat/argo_asf.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

Comments

Andreas Rheinhardt Aug. 8, 2020, 9:46 a.m. UTC | #1
Zane van Iperen:
> Preparation for options.
> 
> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> ---
>  libavformat/argo_asf.c | 42 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/libavformat/argo_asf.c b/libavformat/argo_asf.c
> index 52e37f9c10..1770192aad 100644
> --- a/libavformat/argo_asf.c
> +++ b/libavformat/argo_asf.c
> @@ -292,29 +292,28 @@ static void argo_asf_write_chunk_header(const ArgoASFChunkHeader *ckhdr, AVIOCon
>  
>  static int argo_asf_write_header(AVFormatContext *s)
>  {
> -    const AVCodecParameters  *par = s->streams[0]->codecpar;
> -    ArgoASFFileHeader  fhdr;
> -    ArgoASFChunkHeader chdr;
> -
> -    fhdr.magic         = ASF_TAG;
> -    fhdr.version_major = 2;
> -    fhdr.version_minor = 1;
> -    fhdr.num_chunks    = 1;
> -    fhdr.chunk_offset  = ASF_FILE_HEADER_SIZE;
> -    strncpy(fhdr.name, av_basename(s->url), FF_ARRAY_ELEMS(fhdr.name));
> -
> -    chdr.num_blocks    = 0;
> -    chdr.num_samples   = ASF_SAMPLE_COUNT;
> -    chdr.unk1          = 0;
> -    chdr.sample_rate   = par->sample_rate;
> -    chdr.unk2          = ~0;
> -    chdr.flags         = ASF_CF_BITS_PER_SAMPLE | ASF_CF_ALWAYS1;
> +    const AVCodecParameters *par = s->streams[0]->codecpar;
> +    ArgoASFContext          *ctx = s->priv_data;
> +
> +    ctx->fhdr.magic              = ASF_TAG;
> +    ctx->fhdr.version_major      = 2;
> +    ctx->fhdr.version_minor      = 1;
> +    ctx->fhdr.num_chunks         = 1;
> +    ctx->fhdr.chunk_offset       = ASF_FILE_HEADER_SIZE;
> +    strncpy(ctx->fhdr.name, av_basename(s->url), FF_ARRAY_ELEMS(ctx->fhdr.name));
> +
> +    ctx->ckhdr.num_blocks        = 0;
> +    ctx->ckhdr.num_samples       = ASF_SAMPLE_COUNT;
> +    ctx->ckhdr.unk1              = 0;
> +    ctx->ckhdr.sample_rate       = par->sample_rate;
> +    ctx->ckhdr.unk2              = ~0;
> +    ctx->ckhdr.flags             = ASF_CF_BITS_PER_SAMPLE | ASF_CF_ALWAYS1;
>  
>      if (par->channels == 2)
> -        chdr.flags |= ASF_CF_STEREO;
> +        ctx->ckhdr.flags |= ASF_CF_STEREO;
>  
> -    argo_asf_write_file_header(&fhdr, s->pb);
> -    argo_asf_write_chunk_header(&chdr, s->pb);
> +    argo_asf_write_file_header(&ctx->fhdr, s->pb);
> +    argo_asf_write_chunk_header(&ctx->ckhdr, s->pb);
>      return 0;
>  }
>  
> @@ -353,6 +352,7 @@ AVOutputFormat ff_argo_asf_muxer = {
>      .init           = argo_asf_write_init,
>      .write_header   = argo_asf_write_header,
>      .write_packet   = argo_asf_write_packet,
> -    .write_trailer  = argo_asf_write_trailer
> +    .write_trailer  = argo_asf_write_trailer,
> +    .priv_data_size = sizeof(ArgoASFContext)
>  };
>  #endif
> 
There is no point keeping data only used once in your context which is
not what these contexts are there for. Coupled with patch 4/6 you are
adding a const AVClass * to the context of the demuxer although the
demuxer doesn't use it. 5/6 adds another field unused by the demuxer.
All of this could be better handled by simply using a small, dedicated
structure as context for the muxer.

- Andreas
Zane van Iperen Aug. 8, 2020, 12:34 p.m. UTC | #2
On Sat, 8 Aug 2020 11:46:55 +0200
"Andreas Rheinhardt" <andreas.rheinhardt@gmail.com> wrote:

> >
> There is no point keeping data only used once in your context which is
> not what these contexts are there for. Coupled with patch 4/6 you are
> adding a const AVClass * to the context of the demuxer although the
> demuxer doesn't use it. 5/6 adds another field unused by the demuxer.
> All of this could be better handled by simply using a small, dedicated
> structure as context for the muxer.

Okay, I originally had patches for both, but I wasn't sure what was
preferred. I'll move it back to the dedicated structures. Will probably
make the following patches simpler too...

Zane
diff mbox series

Patch

diff --git a/libavformat/argo_asf.c b/libavformat/argo_asf.c
index 52e37f9c10..1770192aad 100644
--- a/libavformat/argo_asf.c
+++ b/libavformat/argo_asf.c
@@ -292,29 +292,28 @@  static void argo_asf_write_chunk_header(const ArgoASFChunkHeader *ckhdr, AVIOCon
 
 static int argo_asf_write_header(AVFormatContext *s)
 {
-    const AVCodecParameters  *par = s->streams[0]->codecpar;
-    ArgoASFFileHeader  fhdr;
-    ArgoASFChunkHeader chdr;
-
-    fhdr.magic         = ASF_TAG;
-    fhdr.version_major = 2;
-    fhdr.version_minor = 1;
-    fhdr.num_chunks    = 1;
-    fhdr.chunk_offset  = ASF_FILE_HEADER_SIZE;
-    strncpy(fhdr.name, av_basename(s->url), FF_ARRAY_ELEMS(fhdr.name));
-
-    chdr.num_blocks    = 0;
-    chdr.num_samples   = ASF_SAMPLE_COUNT;
-    chdr.unk1          = 0;
-    chdr.sample_rate   = par->sample_rate;
-    chdr.unk2          = ~0;
-    chdr.flags         = ASF_CF_BITS_PER_SAMPLE | ASF_CF_ALWAYS1;
+    const AVCodecParameters *par = s->streams[0]->codecpar;
+    ArgoASFContext          *ctx = s->priv_data;
+
+    ctx->fhdr.magic              = ASF_TAG;
+    ctx->fhdr.version_major      = 2;
+    ctx->fhdr.version_minor      = 1;
+    ctx->fhdr.num_chunks         = 1;
+    ctx->fhdr.chunk_offset       = ASF_FILE_HEADER_SIZE;
+    strncpy(ctx->fhdr.name, av_basename(s->url), FF_ARRAY_ELEMS(ctx->fhdr.name));
+
+    ctx->ckhdr.num_blocks        = 0;
+    ctx->ckhdr.num_samples       = ASF_SAMPLE_COUNT;
+    ctx->ckhdr.unk1              = 0;
+    ctx->ckhdr.sample_rate       = par->sample_rate;
+    ctx->ckhdr.unk2              = ~0;
+    ctx->ckhdr.flags             = ASF_CF_BITS_PER_SAMPLE | ASF_CF_ALWAYS1;
 
     if (par->channels == 2)
-        chdr.flags |= ASF_CF_STEREO;
+        ctx->ckhdr.flags |= ASF_CF_STEREO;
 
-    argo_asf_write_file_header(&fhdr, s->pb);
-    argo_asf_write_chunk_header(&chdr, s->pb);
+    argo_asf_write_file_header(&ctx->fhdr, s->pb);
+    argo_asf_write_chunk_header(&ctx->ckhdr, s->pb);
     return 0;
 }
 
@@ -353,6 +352,7 @@  AVOutputFormat ff_argo_asf_muxer = {
     .init           = argo_asf_write_init,
     .write_header   = argo_asf_write_header,
     .write_packet   = argo_asf_write_packet,
-    .write_trailer  = argo_asf_write_trailer
+    .write_trailer  = argo_asf_write_trailer,
+    .priv_data_size = sizeof(ArgoASFContext)
 };
 #endif