| Message ID | 20161015154056.3148-2-jamrial@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Sat, Oct 15, 2016 at 12:40:55PM -0300, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/matroskadec.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c > index acf1ccb..cfe4692 100644 > --- a/libavformat/matroskadec.c > +++ b/libavformat/matroskadec.c > @@ -1826,6 +1826,10 @@ 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) > + 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); does this need some validity check ? (i didnt immedeatly see any check) << 7 could overflow without check [...]
On 10/17/2016 10:00 PM, Michael Niedermayer wrote: > On Sat, Oct 15, 2016 at 12:40:55PM -0300, James Almer wrote: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/matroskadec.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >> index acf1ccb..cfe4692 100644 >> --- a/libavformat/matroskadec.c >> +++ b/libavformat/matroskadec.c >> @@ -1826,6 +1826,10 @@ 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) >> + 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); > > does this need some validity check ? (i didnt immedeatly see any check) > << 7 could overflow without check avcodec_chroma_pos_to_enum() will return AVCHROMA_LOC_UNSPECIFIED if either xpos or ypos have bad values, so even if it overflows it wouldn't be a problem. The spec doesn't specify a range for the elements, but currently values 0 (Unspecified), 1 (Left/Top Collocated) and 2 (Half) are the only valid ones. I guess 3 may be added in the future, at least for ChromaSitingVert to represent bottom based on our AVChromaLocation enum values. In any case, I'll add some range checks for both elements to be safe. > > [...] > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index acf1ccb..cfe4692 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -1826,6 +1826,10 @@ 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) + 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); if (has_mastering_primaries || has_mastering_luminance) { // Use similar rationals as other standards.
Signed-off-by: James Almer <jamrial@gmail.com> --- libavformat/matroskadec.c | 4 ++++ 1 file changed, 4 insertions(+)