Hi Marton,

Comments inline.

>> +    data = av_packet_get_side_data(pkt, AV_PKT_DATA_AFD, &size);
>> +    if (data) {
>> +        struct klvanc_packet_afd_s *pkt;
>> +        uint16_t *afd;
>> +        uint16_t len;
>> +
>> +        ret = klvanc_create_AFD(&pkt);
>> +        if (ret != 0)
>> +            return AVERROR(ENOMEM);
>> +
>> +        ret = klvanc_set_AFD_val(pkt, data[0]);
>> +        if (ret != 0) {
>> +            av_log(avctx, AV_LOG_ERROR, "Invalid AFD value specified: %d\n",
>> +                   data[0]);
>> +            klvanc_destroy_AFD(pkt);
>> +            return AVERROR(EINVAL);
>> +        }
>> +
>> +        /* FIXME: Should really rely on the coded_width but seems like that
>> +           is not accessible to libavdevice outputs */
>> +        if ((st->codecpar->width == 1280 && st->codecpar->height == 720) ||
>> +            (st->codecpar->width == 1920 && st->codecpar->height == 1080))
>> +            pkt->aspectRatio = ASPECT_16x9;
>> +        else
>> +            pkt->aspectRatio = ASPECT_4x3;
> 
> Does this work for SD 16x9? Shouldn't you also use st->sample_aspect_ratio 
> with some rounding to handle 704x576 16:9 and such mess?

Bear in mind that the “aspectRatio” field in question isn’t what the video 
should ultimately be rendered in.  It’s just the aspect ratio of the source 
video.  For cases where you’re doing SD with 16x9, the source video itself is 
in 4x3 aspect ratio, but the expected result should be in 16x9.  The fact that 
the video should be rendered in 16x9 is controlled by the call to 
klvanc_set_AFD_val() further up in the function.  SAR/PAR, etc aren’t a 
consideration for what drives the aspectRatio field in AFD.

In short, this routine could probably be replaced with something that just 
divides pixelwidth/pixelheight and determines whether it’s equal to 1.77 or 
some smaller value.

I know it’s counter-intuitive to have a detailed aspect ratio description with 
something like 16 possible values, and then to also have a single bit which 
indicates whether the original source video is “16x9” or “4x3” when any 
downstream device can just look at the real pixel width/height of the video.  
Complain to the ATSC, I guess.  :-)

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

Reply via email to