| Message ID | 20180311175854.10096-4-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
On 11/03/18 17:58, James Almer wrote: > There's no need to allocate a new packet for it. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/trace_headers_bsf.c | 30 ++++++++++++++---------------- > 1 file changed, 14 insertions(+), 16 deletions(-) > > diff --git a/libavcodec/trace_headers_bsf.c b/libavcodec/trace_headers_bsf.c > index 93d04cb509..d59bc828a9 100644 > --- a/libavcodec/trace_headers_bsf.c > +++ b/libavcodec/trace_headers_bsf.c > @@ -71,41 +71,39 @@ static int trace_headers(AVBSFContext *bsf, AVPacket *out) > { > TraceHeadersContext *ctx = bsf->priv_data; > CodedBitstreamFragment au; > - AVPacket *in; > char tmp[256] = { 0 }; > int err; > > - err = ff_bsf_get_packet(bsf, &in); > + err = ff_bsf_get_packet_ref(bsf, out); > if (err < 0) > return err; > > - if (in->flags & AV_PKT_FLAG_KEY) > + if (out->flags & AV_PKT_FLAG_KEY) > av_strlcat(tmp, ", key frame", sizeof(tmp)); > - if (in->flags & AV_PKT_FLAG_CORRUPT) > + if (out->flags & AV_PKT_FLAG_CORRUPT) > av_strlcat(tmp, ", corrupt", sizeof(tmp)); > > - if (in->pts != AV_NOPTS_VALUE) > - av_strlcatf(tmp, sizeof(tmp), ", pts %"PRId64, in->pts); > + if (out->pts != AV_NOPTS_VALUE) > + av_strlcatf(tmp, sizeof(tmp), ", pts %"PRId64, out->pts); > else > av_strlcat(tmp, ", no pts", sizeof(tmp)); > - if (in->dts != AV_NOPTS_VALUE) > - av_strlcatf(tmp, sizeof(tmp), ", dts %"PRId64, in->dts); > + if (out->dts != AV_NOPTS_VALUE) > + av_strlcatf(tmp, sizeof(tmp), ", dts %"PRId64, out->dts); > else > av_strlcat(tmp, ", no dts", sizeof(tmp)); > - if (in->duration > 0) > - av_strlcatf(tmp, sizeof(tmp), ", duration %"PRId64, in->duration); > + if (out->duration > 0) > + av_strlcatf(tmp, sizeof(tmp), ", duration %"PRId64, out->duration); > > - av_log(bsf, AV_LOG_INFO, "Packet: %d bytes%s.\n", in->size, tmp); > + av_log(bsf, AV_LOG_INFO, "Packet: %d bytes%s.\n", out->size, tmp); > > - err = ff_cbs_read_packet(ctx->cbc, &au, in); > - if (err < 0) > + err = ff_cbs_read_packet(ctx->cbc, &au, out); > + if (err < 0) { > + av_packet_unref(out); > return err; > + } > > ff_cbs_fragment_uninit(ctx->cbc, &au); > > - av_packet_move_ref(out, in); > - av_packet_free(&in); > - > return 0; > } > > Rename the packet to something like "pkt" throughout? The "out" name looks kindof weird after this change. (That could probably apply to all of the patches, but this one makes the most use of the packet.) Whole series looks fine to me. - Mark
On 3/11/2018 3:32 PM, Mark Thompson wrote: > On 11/03/18 17:58, James Almer wrote: >> There's no need to allocate a new packet for it. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/trace_headers_bsf.c | 30 ++++++++++++++---------------- >> 1 file changed, 14 insertions(+), 16 deletions(-) >> >> diff --git a/libavcodec/trace_headers_bsf.c b/libavcodec/trace_headers_bsf.c >> index 93d04cb509..d59bc828a9 100644 >> --- a/libavcodec/trace_headers_bsf.c >> +++ b/libavcodec/trace_headers_bsf.c >> @@ -71,41 +71,39 @@ static int trace_headers(AVBSFContext *bsf, AVPacket *out) >> { >> TraceHeadersContext *ctx = bsf->priv_data; >> CodedBitstreamFragment au; >> - AVPacket *in; >> char tmp[256] = { 0 }; >> int err; >> >> - err = ff_bsf_get_packet(bsf, &in); >> + err = ff_bsf_get_packet_ref(bsf, out); >> if (err < 0) >> return err; >> >> - if (in->flags & AV_PKT_FLAG_KEY) >> + if (out->flags & AV_PKT_FLAG_KEY) >> av_strlcat(tmp, ", key frame", sizeof(tmp)); >> - if (in->flags & AV_PKT_FLAG_CORRUPT) >> + if (out->flags & AV_PKT_FLAG_CORRUPT) >> av_strlcat(tmp, ", corrupt", sizeof(tmp)); >> >> - if (in->pts != AV_NOPTS_VALUE) >> - av_strlcatf(tmp, sizeof(tmp), ", pts %"PRId64, in->pts); >> + if (out->pts != AV_NOPTS_VALUE) >> + av_strlcatf(tmp, sizeof(tmp), ", pts %"PRId64, out->pts); >> else >> av_strlcat(tmp, ", no pts", sizeof(tmp)); >> - if (in->dts != AV_NOPTS_VALUE) >> - av_strlcatf(tmp, sizeof(tmp), ", dts %"PRId64, in->dts); >> + if (out->dts != AV_NOPTS_VALUE) >> + av_strlcatf(tmp, sizeof(tmp), ", dts %"PRId64, out->dts); >> else >> av_strlcat(tmp, ", no dts", sizeof(tmp)); >> - if (in->duration > 0) >> - av_strlcatf(tmp, sizeof(tmp), ", duration %"PRId64, in->duration); >> + if (out->duration > 0) >> + av_strlcatf(tmp, sizeof(tmp), ", duration %"PRId64, out->duration); >> >> - av_log(bsf, AV_LOG_INFO, "Packet: %d bytes%s.\n", in->size, tmp); >> + av_log(bsf, AV_LOG_INFO, "Packet: %d bytes%s.\n", out->size, tmp); >> >> - err = ff_cbs_read_packet(ctx->cbc, &au, in); >> - if (err < 0) >> + err = ff_cbs_read_packet(ctx->cbc, &au, out); >> + if (err < 0) { >> + av_packet_unref(out); >> return err; >> + } >> >> ff_cbs_fragment_uninit(ctx->cbc, &au); >> >> - av_packet_move_ref(out, in); >> - av_packet_free(&in); >> - >> return 0; >> } >> >> > > Rename the packet to something like "pkt" throughout? The "out" name looks kindof weird after this change. > > (That could probably apply to all of the patches, but this one makes the most use of the packet.) > > Whole series looks fine to me. Changed in all patches and pushed. Thanks! > > - Mark
diff --git a/libavcodec/trace_headers_bsf.c b/libavcodec/trace_headers_bsf.c index 93d04cb509..d59bc828a9 100644 --- a/libavcodec/trace_headers_bsf.c +++ b/libavcodec/trace_headers_bsf.c @@ -71,41 +71,39 @@ static int trace_headers(AVBSFContext *bsf, AVPacket *out) { TraceHeadersContext *ctx = bsf->priv_data; CodedBitstreamFragment au; - AVPacket *in; char tmp[256] = { 0 }; int err; - err = ff_bsf_get_packet(bsf, &in); + err = ff_bsf_get_packet_ref(bsf, out); if (err < 0) return err; - if (in->flags & AV_PKT_FLAG_KEY) + if (out->flags & AV_PKT_FLAG_KEY) av_strlcat(tmp, ", key frame", sizeof(tmp)); - if (in->flags & AV_PKT_FLAG_CORRUPT) + if (out->flags & AV_PKT_FLAG_CORRUPT) av_strlcat(tmp, ", corrupt", sizeof(tmp)); - if (in->pts != AV_NOPTS_VALUE) - av_strlcatf(tmp, sizeof(tmp), ", pts %"PRId64, in->pts); + if (out->pts != AV_NOPTS_VALUE) + av_strlcatf(tmp, sizeof(tmp), ", pts %"PRId64, out->pts); else av_strlcat(tmp, ", no pts", sizeof(tmp)); - if (in->dts != AV_NOPTS_VALUE) - av_strlcatf(tmp, sizeof(tmp), ", dts %"PRId64, in->dts); + if (out->dts != AV_NOPTS_VALUE) + av_strlcatf(tmp, sizeof(tmp), ", dts %"PRId64, out->dts); else av_strlcat(tmp, ", no dts", sizeof(tmp)); - if (in->duration > 0) - av_strlcatf(tmp, sizeof(tmp), ", duration %"PRId64, in->duration); + if (out->duration > 0) + av_strlcatf(tmp, sizeof(tmp), ", duration %"PRId64, out->duration); - av_log(bsf, AV_LOG_INFO, "Packet: %d bytes%s.\n", in->size, tmp); + av_log(bsf, AV_LOG_INFO, "Packet: %d bytes%s.\n", out->size, tmp); - err = ff_cbs_read_packet(ctx->cbc, &au, in); - if (err < 0) + err = ff_cbs_read_packet(ctx->cbc, &au, out); + if (err < 0) { + av_packet_unref(out); return err; + } ff_cbs_fragment_uninit(ctx->cbc, &au); - av_packet_move_ref(out, in); - av_packet_free(&in); - return 0; }
There's no need to allocate a new packet for it. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/trace_headers_bsf.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)