| Message ID | 20200808075914.2296555-5-zane@zanevaniperen.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,1/6] avformat/argo_asf: don't check or probe file version | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Zane van Iperen: > Signed-off-by: Zane van Iperen <zane@zanevaniperen.com> > --- > libavformat/argo_asf.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/libavformat/argo_asf.c b/libavformat/argo_asf.c > index 9845cb955b..3499519903 100644 > --- a/libavformat/argo_asf.c > +++ b/libavformat/argo_asf.c > @@ -69,6 +69,7 @@ typedef struct ArgoASFContext { > ArgoASFFileHeader fhdr; > ArgoASFChunkHeader ckhdr; > uint32_t blocks_read; > + const char *name; > } ArgoASFContext; > > #if CONFIG_ARGO_ASF_DEMUXER > @@ -301,7 +302,10 @@ static int argo_asf_write_header(AVFormatContext *s) > /* version_{major,minor} set by options. */ > 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)); > + if (ctx->name) > + strncpy(ctx->fhdr.name, ctx->name, FF_ARRAY_ELEMS(ctx->fhdr.name)); > + else > + strncpy(ctx->fhdr.name, av_basename(s->url), FF_ARRAY_ELEMS(ctx->fhdr.name)); FF_ARRAY_ELEMS() is actually inappropriate here: Use sizeof() directly. No need to make a separate patch for it. > > ctx->ckhdr.num_blocks = 0; > ctx->ckhdr.num_samples = ASF_SAMPLE_COUNT; > @@ -362,6 +366,14 @@ static const AVOption argo_asf_options[] = { > .max = UINT16_MAX, > .flags = AV_OPT_FLAG_ENCODING_PARAM > }, > + { > + .name = "name", > + .help = "embedded file name (max 8 characters)", > + .offset = offsetof(ArgoASFContext, name), > + .type = AV_OPT_TYPE_STRING, > + .default_val = {.str = NULL}, > + .flags = AV_OPT_FLAG_ENCODING_PARAM > + }, > { NULL } > }; I wonder whether it would not be better to set this via metadata (that the demuxer could export so that it survives a roundtrip). - Andreas
On Sat, 8 Aug 2020 12:20:32 +0200 "Andreas Rheinhardt" <andreas.rheinhardt@gmail.com> wrote: > > #if CONFIG_ARGO_ASF_DEMUXER > > @@ -301,7 +302,10 @@ static int argo_asf_write_header(AVFormatContext *s) > > /* version_{major,minor} set by options. */ > > 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)); > > + if (ctx->name) > > + strncpy(ctx->fhdr.name, ctx->name, FF_ARRAY_ELEMS(ctx->fhdr.name)); > > + else > > + strncpy(ctx->fhdr.name, av_basename(s->url), FF_ARRAY_ELEMS(ctx->fhdr.name)); > > FF_ARRAY_ELEMS() is actually inappropriate here: Use sizeof() directly. Fixed. > No need to make a separate patch for it. Do you mean squash it into the previous one where I add the version options? > > > > ctx->ckhdr.num_blocks = 0; > > ctx->ckhdr.num_samples = ASF_SAMPLE_COUNT; > > @@ -362,6 +366,14 @@ static const AVOption argo_asf_options[] = { > > .max = UINT16_MAX, > > .flags = AV_OPT_FLAG_ENCODING_PARAM > > }, > > + { > > + .name = "name", > > + .help = "embedded file name (max 8 characters)", > > + .offset = offsetof(ArgoASFContext, name), > > + .type = AV_OPT_TYPE_STRING, > > + .default_val = {.str = NULL}, > > + .flags = AV_OPT_FLAG_ENCODING_PARAM > > + }, > > { NULL } > > }; > I wonder whether it would not be better to set this via metadata (that > the demuxer could export so that it survives a roundtrip). Hmmm, possibly? I've only ever seen the field as the filename (minus extension), so I'm not too sure how much value it has as metadata. Zane
Zane van Iperen: > On Sat, 8 Aug 2020 12:20:32 +0200 > "Andreas Rheinhardt" <andreas.rheinhardt@gmail.com> wrote: > >>> #if CONFIG_ARGO_ASF_DEMUXER >>> @@ -301,7 +302,10 @@ static int argo_asf_write_header(AVFormatContext *s) >>> /* version_{major,minor} set by options. */ >>> 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)); >>> + if (ctx->name) >>> + strncpy(ctx->fhdr.name, ctx->name, FF_ARRAY_ELEMS(ctx->fhdr.name)); >>> + else >>> + strncpy(ctx->fhdr.name, av_basename(s->url), FF_ARRAY_ELEMS(ctx->fhdr.name)); >> >> FF_ARRAY_ELEMS() is actually inappropriate here: Use sizeof() directly. > Fixed. > >> No need to make a separate patch for it. > Do you mean squash it into the previous one where I add the version > options? > No, I mean that you don't need to make a separate patch that only changes FF_ARRAY_ELEMS to sizeof. You can directly change it in this patch here. - Andreas
diff --git a/libavformat/argo_asf.c b/libavformat/argo_asf.c index 9845cb955b..3499519903 100644 --- a/libavformat/argo_asf.c +++ b/libavformat/argo_asf.c @@ -69,6 +69,7 @@ typedef struct ArgoASFContext { ArgoASFFileHeader fhdr; ArgoASFChunkHeader ckhdr; uint32_t blocks_read; + const char *name; } ArgoASFContext; #if CONFIG_ARGO_ASF_DEMUXER @@ -301,7 +302,10 @@ static int argo_asf_write_header(AVFormatContext *s) /* version_{major,minor} set by options. */ 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)); + if (ctx->name) + strncpy(ctx->fhdr.name, ctx->name, FF_ARRAY_ELEMS(ctx->fhdr.name)); + else + 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; @@ -362,6 +366,14 @@ static const AVOption argo_asf_options[] = { .max = UINT16_MAX, .flags = AV_OPT_FLAG_ENCODING_PARAM }, + { + .name = "name", + .help = "embedded file name (max 8 characters)", + .offset = offsetof(ArgoASFContext, name), + .type = AV_OPT_TYPE_STRING, + .default_val = {.str = NULL}, + .flags = AV_OPT_FLAG_ENCODING_PARAM + }, { NULL } };
Signed-off-by: Zane van Iperen <zane@zanevaniperen.com> --- libavformat/argo_asf.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)