Re: [libav-devel] [PATCH 2/2] ac3dec: change logging of skipped E-AC-3 substreams.
On Sat, 2 Apr 2016 17:09:50 +0200
Luca Barbato wrote:
> On 02/04/16 14:04, Luca Barbato wrote:
> > On 01/04/16 17:33, James Almer wrote:
> >> On 3/31/2016 10:22 PM, Tim Walker wrote:
> >>> Change log level from warning to debug: the E-AC-3 "core"
> >>> substream can be successfully decoded without the additional
> >>> and dependent substreams, and their presence is already
> >>> indicated via avpriv_request_sample in ff_eac3_parse_header.
> >>> ---
> >>> libavcodec/ac3dec.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
> >>> index 97ce287..1161992 100644
> >>> --- a/libavcodec/ac3dec.c
> >>> +++ b/libavcodec/ac3dec.c
> >>> @@ -1347,7 +1347,7 @@ static int ac3_decode_frame(AVCodecContext * avctx,
> >>> void *data,
> >>> /* skip frame if CRC is ok. otherwise use error concealment.
> >>> */
> >>> /* TODO: add support for substreams and dependent frames */
> >>> if (s->frame_type == EAC3_FRAME_TYPE_DEPENDENT ||
> >>> s->substreamid) {
> >>> -av_log(avctx, AV_LOG_WARNING, "unsupported frame type : "
> >>> +av_log(avctx, AV_LOG_DEBUG, "unsupported frame type : "
> >>> "skipping frame\n");
> >>
> >> Maybe AV_LOG_VERBOSE? Also, it could actually mention what kind of frame
> >> type is being skipped.
> >>
> >
> > I prefer keeping it as debug, reporting the frame type is a good idea
> > though.
> >
>
> Looks like it would be a redundant message if I understood the error
> path correctly.
Don't assume API users are fine with message spam just because e.g.
avconv happens to eliminate redundant messages.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] matroska: Write the field order information
On 03/04/16 11:23, Tim W. wrote:
> On Sat, Apr 2, 2016 at 10:36 PM, Luca Barbato wrote:
>
>> And bump the document version to 4.
>> ---
>> libavformat/matroskaenc.c | 45
>> +++--
>> 1 file changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index c2b1de0..667646d 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -632,6 +632,45 @@ static int mkv_write_codecprivate(AVFormatContext *s,
>> AVIOContext *pb,
>> return ret;
>> }
>>
>> +static void mkv_write_field_order(AVIOContext *pb,
>> + enum AVFieldOrder field_order)
>> +{
>> +switch (field_order) {
>> +case AV_FIELD_UNKNOWN:
>> +put_ebml_uint(pb, MATROSKA_ID_VIDEOFLAGINTERLACED,
>> + MATROSKA_VIDEO_INTERLACE_FLAG_UNDETERMINED);
>> +break;
>> +case AV_FIELD_PROGRESSIVE:
>> +put_ebml_uint(pb, MATROSKA_ID_VIDEOFLAGINTERLACED,
>> + MATROSKA_VIDEO_INTERLACE_FLAG_PROGRESSIVE);
>> +break;
>> +case AV_FIELD_TT:
>> +case AV_FIELD_BB:
>> +case AV_FIELD_TB:
>> +case AV_FIELD_BT:
>> +put_ebml_uint(pb, MATROSKA_ID_VIDEOFLAGINTERLACED,
>> + MATROSKA_VIDEO_INTERLACE_FLAG_INTERLACED);
>> +switch (field_order) {
>> +case AV_FIELD_TT:
>> +put_ebml_uint(pb, MATROSKA_ID_VIDEOFIELDORDER,
>> + MATROSKA_VIDEO_FIELDORDER_TT);
>> +break;
>> +case AV_FIELD_BB:
>> + put_ebml_uint(pb, MATROSKA_ID_VIDEOFIELDORDER,
>> + MATROSKA_VIDEO_FIELDORDER_BB);
>> +break;
>> +case AV_FIELD_TB:
>> +put_ebml_uint(pb, MATROSKA_ID_VIDEOFIELDORDER,
>> + MATROSKA_VIDEO_FIELDORDER_TB);
>> +break;
>> +case AV_FIELD_BT:
>> +put_ebml_uint(pb, MATROSKA_ID_VIDEOFIELDORDER,
>> + MATROSKA_VIDEO_FIELDORDER_BT);
>> +break;
>> +}
>> +}
>> +}
>> +
>> static int mkv_write_stereo_mode(AVFormatContext *s, AVIOContext *pb,
>> AVStream *st, int mode)
>> {
>> @@ -831,10 +870,12 @@ static int mkv_write_track(AVFormatContext *s,
>> MatroskaMuxContext *mkv,
>> }
>>
>> subinfo = start_ebml_master(pb, MATROSKA_ID_TRACKVIDEO, 0);
>> -// XXX: interlace flag?
>> +
>> put_ebml_uint (pb, MATROSKA_ID_VIDEOPIXELWIDTH , par->width);
>> put_ebml_uint (pb, MATROSKA_ID_VIDEOPIXELHEIGHT, par->height);
>>
>> +mkv_write_field_order(pb, par->field_order);
>> +
>> // check both side data and metadata for stereo information,
>> // write the result to the bitstream if any is found
>> ret = mkv_write_stereo_mode(s, pb, st, mkv->mode);
>> @@ -1151,7 +1192,7 @@ static int mkv_write_header(AVFormatContext *s)
>> put_ebml_uint (pb, EBML_ID_EBMLMAXIDLENGTH, 4);
>> put_ebml_uint (pb, EBML_ID_EBMLMAXSIZELENGTH , 8);
>> put_ebml_string (pb, EBML_ID_DOCTYPE, s->oformat->name);
>> -put_ebml_uint (pb, EBML_ID_DOCTYPEVERSION , 2);
>> +put_ebml_uint (pb, EBML_ID_DOCTYPEVERSION , 4);
>> put_ebml_uint (pb, EBML_ID_DOCTYPEREADVERSION , 2);
>> end_ebml_master(pb, ebml_header);
>>
>> --
>> 2.6.1
>>
>
> Looks OK.
The fate hashes need to be updated accordingly.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 2/2] ac3dec: change logging of skipped E-AC-3 substreams.
On Sun, Apr 3, 2016 at 5:17 PM, Luca Barbato wrote: > On 03/04/16 11:26, Tim W. wrote: > > Well, it's definitely redundant before my patchset. Afterwards, the > > avpriv_request_sample is only printed once, so it's (slightly) less > > redundant). > > > > Is there any sort of agreement about what I should do, before I amend the > > patch? > > > > The patch is in, and the idea of the set is having less noise so worst > case can we get a patch by somebody that has a stronger opinion on the > level. Fair enough, thanks. Tim ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 2/2] ac3dec: change logging of skipped E-AC-3 substreams.
On 03/04/16 11:26, Tim W. wrote: > Well, it's definitely redundant before my patchset. Afterwards, the > avpriv_request_sample is only printed once, so it's (slightly) less > redundant). > > Is there any sort of agreement about what I should do, before I amend the > patch? > The patch is in, and the idea of the set is having less noise so worst case can we get a patch by somebody that has a stronger opinion on the level. ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 2/2] ac3dec: change logging of skipped E-AC-3 substreams.
On Sat, Apr 2, 2016 at 5:09 PM, Luca Barbato wrote:
> On 02/04/16 14:04, Luca Barbato wrote:
> > On 01/04/16 17:33, James Almer wrote:
> >> On 3/31/2016 10:22 PM, Tim Walker wrote:
> >>> Change log level from warning to debug: the E-AC-3 "core"
> >>> substream can be successfully decoded without the additional
> >>> and dependent substreams, and their presence is already
> >>> indicated via avpriv_request_sample in ff_eac3_parse_header.
> >>> ---
> >>> libavcodec/ac3dec.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
> >>> index 97ce287..1161992 100644
> >>> --- a/libavcodec/ac3dec.c
> >>> +++ b/libavcodec/ac3dec.c
> >>> @@ -1347,7 +1347,7 @@ static int ac3_decode_frame(AVCodecContext *
> avctx, void *data,
> >>> /* skip frame if CRC is ok. otherwise use error
> concealment. */
> >>> /* TODO: add support for substreams and dependent frames
> */
> >>> if (s->frame_type == EAC3_FRAME_TYPE_DEPENDENT ||
> s->substreamid) {
> >>> -av_log(avctx, AV_LOG_WARNING, "unsupported frame type
> : "
> >>> +av_log(avctx, AV_LOG_DEBUG, "unsupported frame type :
> "
> >>> "skipping frame\n");
> >>
> >> Maybe AV_LOG_VERBOSE? Also, it could actually mention what kind of frame
> >> type is being skipped.
> >>
> >
> > I prefer keeping it as debug, reporting the frame type is a good idea
> > though.
> >
>
> Looks like it would be a redundant message if I understood the error
> path correctly.
>
> lu
Well, it's definitely redundant before my patchset. Afterwards, the
avpriv_request_sample is only printed once, so it's (slightly) less
redundant).
Is there any sort of agreement about what I should do, before I amend the
patch?
Tim
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] vaapi_h264: Fix bit offset of slice data.
On Sat, Apr 2, 2016 at 5:48 PM, Mark Thompson wrote: > Commit ca2f19b9cc37be509d85f05c8f902860475905f8 modified the meaning of > H264SliceContext.gb: it is now initialised at the start of the NAL unit > header, rather than at the start of the slice header. The VAAPI slice > decoder uses the offset after parsing to determine the offset of the > slice data in the bitstream, so with the changed meaning we no longer > need to add the extra byte to account for the NAL unit header because > it is now included directly. > --- > libavcodec/vaapi_h264.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/vaapi_h264.c b/libavcodec/vaapi_h264.c > index 431a428..2e2626e 100644 > --- a/libavcodec/vaapi_h264.c > +++ b/libavcodec/vaapi_h264.c > @@ -328,7 +328,7 @@ static int vaapi_h264_decode_slice(AVCodecContext > *avctx, > slice_param = (VASliceParameterBufferH264 > *)ff_vaapi_alloc_slice(avctx->hwaccel_context, buffer, size); > if (!slice_param) > return -1; > -slice_param->slice_data_bit_offset = get_bits_count(&sl->gb) > + 8; /* bit buffer started beyond nal_unit_type */ > +slice_param->slice_data_bit_offset = get_bits_count(&sl->gb); > slice_param->first_mb_in_slice = (sl->mb_y >> > FIELD_OR_MBAFF_PICTURE(h)) * h->mb_width + sl->mb_x; > slice_param->slice_type = > ff_h264_get_slice_type(sl); > slice_param->direct_spatial_mv_pred_flag= sl->slice_type == > AV_PICTURE_TYPE_B ? sl->direct_spatial_mv_pred : 0; > -- > 2.8.0.rc3 LGTM. Tim ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] matroska: Write the field order information
On Sat, Apr 2, 2016 at 10:36 PM, Luca Barbato wrote:
> And bump the document version to 4.
> ---
> libavformat/matroskaenc.c | 45
> +++--
> 1 file changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index c2b1de0..667646d 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -632,6 +632,45 @@ static int mkv_write_codecprivate(AVFormatContext *s,
> AVIOContext *pb,
> return ret;
> }
>
> +static void mkv_write_field_order(AVIOContext *pb,
> + enum AVFieldOrder field_order)
> +{
> +switch (field_order) {
> +case AV_FIELD_UNKNOWN:
> +put_ebml_uint(pb, MATROSKA_ID_VIDEOFLAGINTERLACED,
> + MATROSKA_VIDEO_INTERLACE_FLAG_UNDETERMINED);
> +break;
> +case AV_FIELD_PROGRESSIVE:
> +put_ebml_uint(pb, MATROSKA_ID_VIDEOFLAGINTERLACED,
> + MATROSKA_VIDEO_INTERLACE_FLAG_PROGRESSIVE);
> +break;
> +case AV_FIELD_TT:
> +case AV_FIELD_BB:
> +case AV_FIELD_TB:
> +case AV_FIELD_BT:
> +put_ebml_uint(pb, MATROSKA_ID_VIDEOFLAGINTERLACED,
> + MATROSKA_VIDEO_INTERLACE_FLAG_INTERLACED);
> +switch (field_order) {
> +case AV_FIELD_TT:
> +put_ebml_uint(pb, MATROSKA_ID_VIDEOFIELDORDER,
> + MATROSKA_VIDEO_FIELDORDER_TT);
> +break;
> +case AV_FIELD_BB:
> + put_ebml_uint(pb, MATROSKA_ID_VIDEOFIELDORDER,
> + MATROSKA_VIDEO_FIELDORDER_BB);
> +break;
> +case AV_FIELD_TB:
> +put_ebml_uint(pb, MATROSKA_ID_VIDEOFIELDORDER,
> + MATROSKA_VIDEO_FIELDORDER_TB);
> +break;
> +case AV_FIELD_BT:
> +put_ebml_uint(pb, MATROSKA_ID_VIDEOFIELDORDER,
> + MATROSKA_VIDEO_FIELDORDER_BT);
> +break;
> +}
> +}
> +}
> +
> static int mkv_write_stereo_mode(AVFormatContext *s, AVIOContext *pb,
> AVStream *st, int mode)
> {
> @@ -831,10 +870,12 @@ static int mkv_write_track(AVFormatContext *s,
> MatroskaMuxContext *mkv,
> }
>
> subinfo = start_ebml_master(pb, MATROSKA_ID_TRACKVIDEO, 0);
> -// XXX: interlace flag?
> +
> put_ebml_uint (pb, MATROSKA_ID_VIDEOPIXELWIDTH , par->width);
> put_ebml_uint (pb, MATROSKA_ID_VIDEOPIXELHEIGHT, par->height);
>
> +mkv_write_field_order(pb, par->field_order);
> +
> // check both side data and metadata for stereo information,
> // write the result to the bitstream if any is found
> ret = mkv_write_stereo_mode(s, pb, st, mkv->mode);
> @@ -1151,7 +1192,7 @@ static int mkv_write_header(AVFormatContext *s)
> put_ebml_uint (pb, EBML_ID_EBMLMAXIDLENGTH, 4);
> put_ebml_uint (pb, EBML_ID_EBMLMAXSIZELENGTH , 8);
> put_ebml_string (pb, EBML_ID_DOCTYPE, s->oformat->name);
> -put_ebml_uint (pb, EBML_ID_DOCTYPEVERSION , 2);
> +put_ebml_uint (pb, EBML_ID_DOCTYPEVERSION , 4);
> put_ebml_uint (pb, EBML_ID_DOCTYPEREADVERSION , 2);
> end_ebml_master(pb, ebml_header);
>
> --
> 2.6.1
>
Looks OK.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
