[FFmpeg-cvslog] matroska: cleanup handling of video stereo mode

Reimar Döffinger Reimar.Doeffinger at gmx.de
Tue May 24 20:15:32 CEST 2011


On Tue, May 24, 2011 at 01:13:35AM +0200, Aurelien Jacobs wrote:
> +const char const *matroska_video_stereo_mode[] = {
> +const char const *matroska_video_stereo_plane[] = {

That's wrong.
"const char *", "char const *" and "const char const *"
are all exactly the same thing.
You meant "const char * const".

> +#define MATROSKA_VIDEO_STEREO_MODE_COUNT  15
> +#define MATROSKA_VIDEO_STEREO_PLANE_COUNT  3

If you use defines, you should also use those defines
in the array declarations, that avoids overreads and will
also remind people that they have to change them when changing
the array.

> +            /* export stereo mode flag as metadata tag */
> +            if (track->video.stereo_mode && track->video.stereo_mode < MATROSKA_VIDEO_STEREO_MODE_COUNT)
> +                av_metadata_set2(&st->metadata, "stereo_mode", matroska_video_stereo_mode[track->video.stereo_mode], 0);

It might be a good idea to warn in case of an unknown value.

> +            /* if we have virtual track, mark the real tracks */
> +            for (j=0; j < track->operation.combine_planes.nb_elem; j++) {
> +                char buf[32];
> +                if (planes[j].type < MATROSKA_VIDEO_STEREO_PLANE_COUNT)
> +                    continue;

Huh? Did you flip the condition?!


More information about the ffmpeg-cvslog mailing list