Re: [FFmpeg-devel] [PATCH 2/3 v2] avformat/matroskadec: support parsing Chroma Location elements

2016-10-18 Thread James Almer 
On 10/18/2016 11:10 AM, Michael Niedermayer wrote:
> On Tue, Oct 18, 2016 at 12:18:01AM -0300, James Almer wrote:
>> Signed-off-by: James Almer 
>> ---
>>  libavformat/matroska.h| 14 ++
>>  libavformat/matroskadec.c |  7 +++
>>  2 files changed, 21 insertions(+)
>>
>> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
>> index 8ad89da..13155e5 100644
>> --- a/libavformat/matroska.h
>> +++ b/libavformat/matroska.h
>> @@ -317,6 +317,20 @@ typedef enum {
>>MATROSKA_VIDEO_DISPLAYUNIT_UNKNOWN = 4,
>>  } MatroskaVideoDisplayUnit;
>>  
>> +typedef enum {
>> +  MATROSKA_COLOUR_CHROMASITINGHORZ_UNDETERMINED = 0,
>> +  MATROSKA_COLOUR_CHROMASITINGHORZ_LEFT = 1,
>> +  MATROSKA_COLOUR_CHROMASITINGHORZ_HALF = 2,
>> +  MATROSKA_COLOUR_CHROMASITINGHORZ_NB
>> +} MatroskaColourChromaSitingHorz;
>> +
>> +typedef enum {
>> +  MATROSKA_COLOUR_CHROMASITINGVERT_UNDETERMINED = 0,
>> +  MATROSKA_COLOUR_CHROMASITINGVERT_TOP  = 1,
>> +  MATROSKA_COLOUR_CHROMASITINGVERT_HALF = 2,
>> +  MATROSKA_COLOUR_CHROMASITINGVERT_NB
>> +} MatroskaColourChromaSitingVert;
>> +
>>  /*
>>   * Matroska Codec IDs, strings
>>   */
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index a5d3c0e..722d0b0 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -1827,6 +1827,13 @@ static int mkv_parse_video_color(AVStream *st, const 
>> MatroskaTrack *track) {
>>  if (track->video.color.range != AVCOL_RANGE_UNSPECIFIED &&
>>  track->video.color.range <= AVCOL_RANGE_JPEG)
>>  st->codecpar->color_range = track->video.color.range;
>> +if (track->video.color.chroma_siting_horz && 
>> track->video.color.chroma_siting_vert &&
>> +track->video.color.chroma_siting_horz < 
>> MATROSKA_COLOUR_CHROMASITINGHORZ_NB &&
>> +track->video.color.chroma_siting_vert < 
>> MATROSKA_COLOUR_CHROMASITINGVERT_NB) {
>> +st->codecpar->chroma_location =
>> +
>> avcodec_chroma_pos_to_enum((track->video.color.chroma_siting_horz - 1) << 7,
>> +   
>> (track->video.color.chroma_siting_vert - 1) << 7);
>> +}
> 
> on the assumtation that the cases match up correctly, (I did not
> cross-check the spec)

The values in the enums above are taken from the spec.

In any case this code is still marked as "unofficial", even though all
the elements are already listed in the spec page and git repo.
Could someone more familiar with the current standardization process
chime in to confirm if these Colour elements are finalized or not? The
unofficial compliance check could be removed if so.

> , LGTM
> thx

Pushed, thanks.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/3 v2] avformat/matroskadec: support parsing Chroma Location elements

2016-10-18 Thread Michael Niedermayer 
On Tue, Oct 18, 2016 at 12:18:01AM -0300, James Almer wrote:
> Signed-off-by: James Almer 
> ---
>  libavformat/matroska.h| 14 ++
>  libavformat/matroskadec.c |  7 +++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
> index 8ad89da..13155e5 100644
> --- a/libavformat/matroska.h
> +++ b/libavformat/matroska.h
> @@ -317,6 +317,20 @@ typedef enum {
>MATROSKA_VIDEO_DISPLAYUNIT_UNKNOWN = 4,
>  } MatroskaVideoDisplayUnit;
>  
> +typedef enum {
> +  MATROSKA_COLOUR_CHROMASITINGHORZ_UNDETERMINED = 0,
> +  MATROSKA_COLOUR_CHROMASITINGHORZ_LEFT = 1,
> +  MATROSKA_COLOUR_CHROMASITINGHORZ_HALF = 2,
> +  MATROSKA_COLOUR_CHROMASITINGHORZ_NB
> +} MatroskaColourChromaSitingHorz;
> +
> +typedef enum {
> +  MATROSKA_COLOUR_CHROMASITINGVERT_UNDETERMINED = 0,
> +  MATROSKA_COLOUR_CHROMASITINGVERT_TOP  = 1,
> +  MATROSKA_COLOUR_CHROMASITINGVERT_HALF = 2,
> +  MATROSKA_COLOUR_CHROMASITINGVERT_NB
> +} MatroskaColourChromaSitingVert;
> +
>  /*
>   * Matroska Codec IDs, strings
>   */
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index a5d3c0e..722d0b0 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1827,6 +1827,13 @@ static int mkv_parse_video_color(AVStream *st, const 
> MatroskaTrack *track) {
>  if (track->video.color.range != AVCOL_RANGE_UNSPECIFIED &&
>  track->video.color.range <= AVCOL_RANGE_JPEG)
>  st->codecpar->color_range = track->video.color.range;
> +if (track->video.color.chroma_siting_horz && 
> track->video.color.chroma_siting_vert &&
> +track->video.color.chroma_siting_horz < 
> MATROSKA_COLOUR_CHROMASITINGHORZ_NB &&
> +track->video.color.chroma_siting_vert < 
> MATROSKA_COLOUR_CHROMASITINGVERT_NB) {
> +st->codecpar->chroma_location =
> +
> avcodec_chroma_pos_to_enum((track->video.color.chroma_siting_horz - 1) << 7,
> +   
> (track->video.color.chroma_siting_vert - 1) << 7);
> +}

on the assumtation that the cases match up correctly, (I did not
cross-check the spec), LGTM

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel