On 19 Feb 2014, at 21:16, Anton Khirnov <an...@khirnov.net> wrote:

> ---
> libavfilter/af_ashowinfo.c |   91 ++++++++++++++++++++++++++++++++++++++++++++
> libavfilter/vf_showinfo.c  |   54 ++++++++++++++++++++++++++
> 2 files changed, 145 insertions(+)

Nice! :-)

> 
> diff --git a/libavfilter/af_ashowinfo.c b/libavfilter/af_ashowinfo.c
> index 2a2edcf..9d36d5e 100644
> --- a/libavfilter/af_ashowinfo.c
> +++ b/libavfilter/af_ashowinfo.c
> @@ -30,6 +30,8 @@
> #include "libavutil/attributes.h"
> #include "libavutil/channel_layout.h"
> #include "libavutil/common.h"
> +#include "libavutil/downmix_info.h"
> +#include "libavutil/intreadwrite.h"
> #include "libavutil/mem.h"
> #include "libavutil/samplefmt.h"
> 
> @@ -66,6 +68,81 @@ static av_cold void uninit(AVFilterContext *ctx)
>     av_freep(&s->plane_checksums);
> }
> 
> +static void dump_matrixenc(AVFilterContext *ctx, AVFrameSideData *sd)
> +{
> +    enum AVMatrixEncoding enc;
> +
> +    av_log(ctx, AV_LOG_INFO, "matrix encoding: ");
> +
> +    if (sd->size < sizeof(enum AVMatrixEncoding)) {
> +        av_log(ctx, AV_LOG_INFO, "invalid data");
> +        return;
> +    }
> +
> +    enc = *(enum AVMatrixEncoding *)sd->data;
> +    switch (enc) {
> +    case AV_MATRIX_ENCODING_NONE:           av_log(ctx, AV_LOG_INFO, 
> "none");            break;
> +    case AV_MATRIX_ENCODING_DOLBY:          av_log(ctx, AV_LOG_INFO, 
> "Dolby");           break;
> +    case AV_MATRIX_ENCODING_DPLII:          av_log(ctx, AV_LOG_INFO, 
> "DPLII");           break;
> +    case AV_MATRIX_ENCODING_DPLIIX:         av_log(ctx, AV_LOG_INFO, 
> "DPLIIX");          break;
> +    case AV_MATRIX_ENCODING_DPLIIZ:         av_log(ctx, AV_LOG_INFO, 
> "DPLIIZ");          break;

Maybe DPLIIx, DPLIIz? The "official" name is Dolby Pro Logic IIx/IIz.

> +    case AV_MATRIX_ENCODING_DOLBYEX:        av_log(ctx, AV_LOG_INFO, "Dolby 
> EX");        break;
> +    case AV_MATRIX_ENCODING_DOLBYHEADPHONE: av_log(ctx, AV_LOG_INFO, "Dolby 
> headphone"); break;
> +    default:                                av_log(ctx, AV_LOG_INFO, 
> "unknown");         break;

Nit: I don't really like how some values start with a capital letter and others 
not.

We could either go with "None", "Unknown" and maybe even "Dolby Headphone", or 
go all lowercase (like you did for Stereo3D, for all except "2D"). Thoughts?

> +    }
> +}
> +
> +static void dump_downmix(AVFilterContext *ctx, AVFrameSideData *sd)
> +{
> +    AVDownmixInfo *di;
> +
> +    av_log(ctx, AV_LOG_INFO, "downmix: ");
> +    if (sd->size < sizeof(*di)) {
> +        av_log(ctx, AV_LOG_INFO, "invalid data");
> +        return;
> +    }
> +
> +    di = (AVDownmixInfo *)sd->data;
> +
> +    av_log(ctx, AV_LOG_INFO, "preferred downmix type - ");
> +    switch (di->preferred_downmix_type) {
> +    case AV_DOWNMIX_TYPE_LORO:    av_log(ctx, AV_LOG_INFO, "LORO");    break;
> +    case AV_DOWNMIX_TYPE_LTRT:    av_log(ctx, AV_LOG_INFO, "LTRT");    break;

Maybe "Lo/Ro", "Lt/Rt"?

> +    case AV_DOWNMIX_TYPE_DPLII:   av_log(ctx, AV_LOG_INFO, "DPLII");   break;
> +    default:                      av_log(ctx, AV_LOG_INFO, "unknown"); break;

Same as above re: case.

> +    }
> +
> +    av_log(ctx, AV_LOG_INFO, "center mix level - %f (%f ltrt) "
> +           "surround mix level - %f (%f ltrt) lfe mix level: %f",
> +           di->center_mix_level, di->center_mix_level_ltrt,
> +           di->surround_mix_level, di->surround_mix_level_ltrt,
> +           di->lfe_mix_level);
> +}

Maybe "Mix levels: center %f (%f ltrt) - surround %f (%f ltrt) - LFE %f"?

Looks fine as is though.

> +
> +static void dump_replaygain(AVFilterContext *ctx, AVFrameSideData *sd)
> +{
> +    float tg, tp, ag, ap;
> +
> +    av_log(ctx, AV_LOG_INFO, "replaygain: ");
> +    if (sd->size < 16) {
> +        av_log(ctx, AV_LOG_INFO, "invalid data");
> +        return;
> +    }
> +
> +    tg = (int32_t)AV_RL32(sd->data) / 100000.0f;
> +    tp = (float)AV_RL32(sd->data + 4) / UINT32_MAX;
> +    ag = (int32_t)AV_RL32(sd->data + 8) / 100000.0f;
> +    ap = (float)AV_RL32(sd->data + 12) / UINT32_MAX;

Nit: a bit more alignment wouldn't hurt.

> +
> +    av_log(ctx, AV_LOG_INFO, "track gain - %f dB, track peak - %f, album 
> gain - %f dB, album peak - %f",
> +           tg, tp, ag, ap);
> +}
> +
> +static void dump_unknown(AVFilterContext *ctx, AVFrameSideData *sd)
> +{
> +    av_log(ctx, AV_LOG_INFO, "unknown side data type: %d, size %d bytes", 
> sd->type, sd->size);
> +}
> +
> static int filter_frame(AVFilterLink *inlink, AVFrame *buf)
> {
>     AVFilterContext *ctx = inlink->dst;
> @@ -104,6 +181,20 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> *buf)
>         av_log(ctx, AV_LOG_INFO, "%08X ", s->plane_checksums[i]);
>     av_log(ctx, AV_LOG_INFO, "]\n");
> 
> +    for (i = 0; i < buf->nb_side_data; i++) {
> +        AVFrameSideData *sd = buf->side_data[i];
> +
> +        av_log(ctx, AV_LOG_INFO, "  side data - ");
> +        switch (sd->type) {
> +        case AV_FRAME_DATA_MATRIXENCODING: dump_matrixenc (ctx, sd); break;
> +        case AV_FRAME_DATA_DOWNMIX_INFO:   dump_downmix   (ctx, sd); break;
> +        case AV_FRAME_DATA_REPLAYGAIN:     dump_replaygain(ctx, sd); break;
> +        default:                           dump_unknown   (ctx, sd); break;
> +        }
> +
> +        av_log(ctx, AV_LOG_INFO, "\n");
> +    }
> +
>     s->frame++;
>     return ff_filter_frame(inlink->dst->outputs[0], buf);
> }
> diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> index e89ffe0..412d397 100644
> --- a/libavfilter/vf_showinfo.c
> +++ b/libavfilter/vf_showinfo.c
> @@ -26,6 +26,8 @@
> #include "libavutil/imgutils.h"
> #include "libavutil/internal.h"
> #include "libavutil/pixdesc.h"
> +#include "libavutil/stereo3d.h"
> +
> #include "avfilter.h"
> #include "internal.h"
> #include "video.h"
> @@ -34,6 +36,35 @@ typedef struct {
>     unsigned int frame;
> } ShowInfoContext;
> 
> +static void dump_stereo3d(AVFilterContext *ctx, AVFrameSideData *sd)
> +{
> +    AVStereo3D *stereo;
> +
> +    av_log(ctx, AV_LOG_INFO, "stereoscopic information: ");
> +    if (sd->size < sizeof(*stereo)) {
> +        av_log(ctx, AV_LOG_INFO, "invalid data");
> +        return;
> +    }
> +
> +    stereo = (AVStereo3D *)sd->data;
> +
> +    av_log(ctx, AV_LOG_INFO, "type - ");
> +    switch (stereo->type) {
> +    case AV_STEREO3D_2D:                  av_log(ctx, AV_LOG_INFO, "2D");    
>                 break;

Only uppercase character in the whole function. Yet it makes sense, so TBH I 
don't know what to suggest.

> +    case AV_STEREO3D_SIDEBYSIDE:          av_log(ctx, AV_LOG_INFO, "side by 
> side");          break;
> +    case AV_STEREO3D_TOPBOTTOM:           av_log(ctx, AV_LOG_INFO, 
> "top/bottom");            break;
> +    case AV_STEREO3D_FRAMESEQUENCE:       av_log(ctx, AV_LOG_INFO, "frame 
> sequence");        break;
> +    case AV_STEREO3D_CHECKERBOARD:        av_log(ctx, AV_LOG_INFO, 
> "checkerboard");          break;
> +    case AV_STEREO3D_LINES:               av_log(ctx, AV_LOG_INFO, 
> "interleaved lines");     break;
> +    case AV_STEREO3D_COLUMNS:             av_log(ctx, AV_LOG_INFO, 
> "interleaved columns");   break;
> +    case AV_STEREO3D_SIDEBYSIDE_QUINCUNX: av_log(ctx, AV_LOG_INFO, "side by 
> side quincunx"); break;
> +    default:                              av_log(ctx, AV_LOG_INFO, 
> "unknown");               break;
> +    }
> +
> +    if (stereo->flags & AV_STEREO3D_FLAG_INVERT)
> +        av_log(ctx, AV_LOG_INFO, " (inverted)");
> +}
> +
> static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
> {
>     AVFilterContext *ctx = inlink->dst;
> @@ -69,6 +100,29 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> *frame)
>            av_get_picture_type_char(frame->pict_type),
>            checksum, plane_checksum[0], plane_checksum[1], plane_checksum[2], 
> plane_checksum[3]);
> 
> +    for (i = 0; i < frame->nb_side_data; i++) {
> +        AVFrameSideData *sd = frame->side_data[i];
> +
> +        av_log(ctx, AV_LOG_INFO, "  side data - ");
> +        switch (sd->type) {
> +        case AV_FRAME_DATA_PANSCAN:
> +            av_log(ctx, AV_LOG_INFO, "pan/scan");
> +            break;
> +        case AV_FRAME_DATA_A53_CC:
> +            av_log(ctx, AV_LOG_INFO, "A53 closed captions (%d bytes)", 
> sd->size);

Again, only uppercase in this function. Also, the standard would be "A/53", 
like "A/52".

Tim
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to