Just some small observations... Even if you postpone this patch,
perhaps it can help improve the next one.
On Thu, Jul 06, 2017 at 17:36:39 -0700, Louis O'Bryan wrote:
> @@ -2,6 +2,7 @@ Entries are sorted chronologically from oldest to youngest
> within each release,
> releases are sorted from youngest to oldest.
>
> version <next>:
> +- Camera metadata motion encoder
> - deflicker video filter
Did you read that sentence above? ;-) New entries at the bottom of the
"version <next>" section please.
> @item Zip Motion Blocks Video @tab X @tab X
> @tab Encoder works only in PAL8.
> +@item Camera metadata motion @tab X @tab
> + @tab Encoder for camera sensor data.
This list is supposed to be alphabetical.
> OBJS-$(CONFIG_ZLIB_ENCODER) += lclenc.o
> OBJS-$(CONFIG_ZMBV_DECODER) += zmbv.o
> OBJS-$(CONFIG_ZMBV_ENCODER) += zmbvenc.o
> +OBJS-$(CONFIG_CAMERA_MOTION_METADATA_ENCODER) += cammenc.o
This also needs to be in alphabetical order.
> + * Reference implementation for the CAMM Metadata encoder.
> + * Encodes sensor data for 360-degree cameras such as
> + * GPS, gyro, and acceleration. This is stored in a track separate from video
> + * and audio.
I believe you're just supposed to have a short entry here, the more
detailed stuff goes into the section below the GPL boilerplate.
> +// Sizes of each type of metadata.
> +static int metadata_type_sizes[] = {
> + 3 * sizeof(float),
> + 2 * sizeof(uint64_t),
> + 3 * sizeof(float),
> + 3 * sizeof(float),
> + 3 * sizeof(float),
> + 3 * sizeof(float),
> + 3 * sizeof(double) + sizeof(uint32_t) + 7 * sizeof(float),
> + 3 * sizeof(float)
> +};
Doesn't this make the sizes platform dependant? (Unless all platforms
have standardized floats anyway, then I'm mistaken. I just don't know.)
> + av_log(avctx, AV_LOG_DEBUG,
> + "pixel_exposure_time = %lu\n", pixel_exposure_time);
pixel_exposure_time is a uint64_t, so '%lu' is incorrect, it's
'%"PRIu64"'. (Elsewhere as well.) The <stdint.h> types have theitr own
format identifiers.
> + av_log(avctx, AV_LOG_DEBUG, "rolling_shutter_skew_time = %lu\n",
> + rolling_shutter_skew_time);
Ditto.
> + if (gps_fix_type != 0 && gps_fix_type != 1 && gps_fix_type != 2)
> {
You could do "gps_fix_type > 2", but since it's sort of an enum, I
guess this is fine.
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
This does NOT belong into your patch! (Or only parts of it.) You copied
some modified code over a recent git checkout, thereby apparently
reverting some unrelated recent commits. I recommend you checkout/pull,
modify only you stuff, and then commit locally. That keeps your changes
constrained, even if the code changes upstream.
Please re-apply your changes to a recent checkout.
Moritz
_______________________________________________
ffmpeg-devel mailing list
[email protected]
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
