From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 428974A9A7 for ; Fri, 5 Jul 2024 20:50:45 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 4B40168DBA2; Fri, 5 Jul 2024 23:50:43 +0300 (EEST) Received: from mail-pl1-f179.google.com (mail-pl1-f179.google.com [209.85.214.179]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 93BC368DB86 for ; Fri, 5 Jul 2024 23:50:36 +0300 (EEST) Received: by mail-pl1-f179.google.com with SMTP id d9443c01a7336-1fb0d7e4ee9so12872995ad.3 for ; Fri, 05 Jul 2024 13:50:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1720212634; x=1720817434; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=VzbaBcUpZPBoB17uDZCVyx8sW2b1B69JHITJjs+GEYs=; b=F8YmATW79KdoPkaYdMMvwn0vqmcDLoQ+QJGene7UpiJBs7h5EmOdSUwbBdp6lJiRez m7KIaP9EStmKs+Y0Sk7K5rT+CL/HmAzzNwgOg6L4qNmkVG8YyoL8jbQqFadNt0zRPC0S /Q3NrBPjfQ+bl/DjMP3YYSLQxCKmn16NTojKH7ZnBu5UGn39WhylgrnC89YjujOn76Tg HAA7TA849adgtolmgKAce4n0TtKZuyl0C5dBda+W83NJc2Jhb45yd/d/A5Pp/PYqCVXW 6xoaPVA37Q10TXFzgoOCyPwdGA5WNnaN/8ZGwm2JXHgTzoIZCySfLvAB7uSwfMCP8kgN OFxQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720212634; x=1720817434; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=VzbaBcUpZPBoB17uDZCVyx8sW2b1B69JHITJjs+GEYs=; b=FFuNqAsU/0rJFrTXVNg+oHYoKEadarfkNrXvcm17N3hR80StldI2R92h5m2y+EHFbL 9GYu6SaVfohI1pNKvg3fn3LCNAPoGDmcqXntlPtFwhSIxy4HrZPlevoFCDHFRODvP9fZ bLWCtQIcLdVFBCt5Efm9nayLCg4sznq1goc01yYQxlsW80BwwdO91xb/xo+c8/u8AR+F WEh2FqYTo0cEmzaw3CPPFwZz4uYiqsQNBbHXJE5jkS2aUcyqT4FMhWBZxOPPG9m5gKrI zmuT47zeV2U3PPwOPsclARbvwepIjrHsg8hc6IUtbcs8XPgur0zug8Dr/tQU4hWAwP0g kL/g== X-Gm-Message-State: AOJu0YzMPbLgvgyqn2OE/XCC7U2q6DQWfnkmTei1c5UhI16FGmcV3jDU OFvwO1fm5lEheUnDwq0Di98kldy4+GIWrQ39RX9t8n8TuhmtmSzLaBsJrA== X-Google-Smtp-Source: AGHT+IGX9n81d/77ZYEPnQtuS7+m0M/RXN99GRl0DLR1PQdL1ZNZbyaCICjC8mVUdfnd8tPSJT2qQg== X-Received: by 2002:a17:903:2352:b0:1fb:325d:2b39 with SMTP id d9443c01a7336-1fb33e9854cmr47859965ad.30.1720212633633; Fri, 05 Jul 2024 13:50:33 -0700 (PDT) Received: from [192.168.0.16] ([190.194.167.233]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1fb38ad6d9csm33223395ad.243.2024.07.05.13.50.32 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 05 Jul 2024 13:50:33 -0700 (PDT) Message-ID: <275f5bf7-dce1-44d7-90a2-52107439f5cc@gmail.com> Date: Fri, 5 Jul 2024 17:51:09 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20240529214632.9843-1-jamrial@gmail.com> <20240529214632.9843-3-jamrial@gmail.com> Content-Language: en-US From: James Almer In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH 3/6] avformat/matroskadec: export cropping values X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 6/1/2024 8:24 AM, Kacper Michajlow wrote: > On Wed, 29 May 2024 at 23:47, James Almer wrote: >> >> Signed-off-by: James Almer >> --- >> libavformat/matroskadec.c | 53 +++++++++++++++++++++++++++++++-------- >> 1 file changed, 43 insertions(+), 10 deletions(-) >> >> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >> index 2f07e11d87..a30bac786b 100644 >> --- a/libavformat/matroskadec.c >> +++ b/libavformat/matroskadec.c >> @@ -213,7 +213,13 @@ typedef struct MatroskaTrackVideo { >> uint64_t display_height; >> uint64_t pixel_width; >> uint64_t pixel_height; >> + uint64_t cropped_width; >> + uint64_t cropped_height; >> EbmlBin color_space; >> + uint64_t pixel_cropt; >> + uint64_t pixel_cropl; >> + uint64_t pixel_cropb; >> + uint64_t pixel_cropr; >> uint64_t display_unit; >> uint64_t interlaced; >> uint64_t field_order; >> @@ -527,10 +533,10 @@ static EbmlSyntax matroska_track_video[] = { >> { MATROSKA_ID_VIDEOALPHAMODE, EBML_UINT, 0, 0, offsetof(MatroskaTrackVideo, alpha_mode), { .u = 0 } }, >> { MATROSKA_ID_VIDEOCOLOR, EBML_NEST, 0, sizeof(MatroskaTrackVideoColor), offsetof(MatroskaTrackVideo, color), { .n = matroska_track_video_color } }, >> { MATROSKA_ID_VIDEOPROJECTION, EBML_NEST, 0, 0, offsetof(MatroskaTrackVideo, projection), { .n = matroska_track_video_projection } }, >> - { MATROSKA_ID_VIDEOPIXELCROPB, EBML_NONE }, >> - { MATROSKA_ID_VIDEOPIXELCROPT, EBML_NONE }, >> - { MATROSKA_ID_VIDEOPIXELCROPL, EBML_NONE }, >> - { MATROSKA_ID_VIDEOPIXELCROPR, EBML_NONE }, >> + { MATROSKA_ID_VIDEOPIXELCROPB, EBML_UINT, 0, 0, offsetof(MatroskaTrackVideo, pixel_cropb), {.u = 0 } }, >> + { MATROSKA_ID_VIDEOPIXELCROPT, EBML_UINT, 0, 0, offsetof(MatroskaTrackVideo, pixel_cropt), {.u = 0 } }, >> + { MATROSKA_ID_VIDEOPIXELCROPL, EBML_UINT, 0, 0, offsetof(MatroskaTrackVideo, pixel_cropl), {.u = 0 } }, >> + { MATROSKA_ID_VIDEOPIXELCROPR, EBML_UINT, 0, 0, offsetof(MatroskaTrackVideo, pixel_cropr), {.u = 0 } }, >> { MATROSKA_ID_VIDEODISPLAYUNIT, EBML_UINT, 0, 0, offsetof(MatroskaTrackVideo, display_unit), { .u= MATROSKA_VIDEO_DISPLAYUNIT_PIXELS } }, >> { MATROSKA_ID_VIDEOFLAGINTERLACED, EBML_UINT, 0, 0, offsetof(MatroskaTrackVideo, interlaced), { .u = MATROSKA_VIDEO_INTERLACE_FLAG_UNDETERMINED } }, >> { MATROSKA_ID_VIDEOFIELDORDER, EBML_UINT, 0, 0, offsetof(MatroskaTrackVideo, field_order), { .u = MATROSKA_VIDEO_FIELDORDER_UNDETERMINED } }, >> @@ -2963,14 +2969,30 @@ static int mkv_parse_video(MatroskaTrack *track, AVStream *st, >> >> if (track->video.display_unit < MATROSKA_VIDEO_DISPLAYUNIT_UNKNOWN) { >> if (track->video.display_width && track->video.display_height && >> - par->height < INT64_MAX / track->video.display_width / display_width_mul && >> - par->width < INT64_MAX / track->video.display_height / display_height_mul) >> + track->video.cropped_height < INT64_MAX / track->video.display_width / display_width_mul && >> + track->video.cropped_width < INT64_MAX / track->video.display_height / display_height_mul) >> av_reduce(&st->sample_aspect_ratio.num, >> &st->sample_aspect_ratio.den, >> - par->height * track->video.display_width * display_width_mul, >> - par->width * track->video.display_height * display_height_mul, >> + track->video.cropped_height * track->video.display_width * display_width_mul, >> + track->video.cropped_width * track->video.display_height * display_height_mul, >> INT_MAX); >> } >> + if (track->video.cropped_width != track->video.pixel_width || >> + track->video.cropped_height != track->video.pixel_height) { >> + uint8_t *cropping; >> + AVPacketSideData *sd = av_packet_side_data_new(&st->codecpar->coded_side_data, >> + &st->codecpar->nb_coded_side_data, >> + AV_PKT_DATA_FRAME_CROPPING, >> + sizeof(uint32_t) * 4, 0); >> + if (!sd) >> + return AVERROR(ENOMEM); >> + >> + cropping = sd->data; >> + bytestream_put_le32(&cropping, track->video.pixel_cropt); >> + bytestream_put_le32(&cropping, track->video.pixel_cropb); >> + bytestream_put_le32(&cropping, track->video.pixel_cropl); >> + bytestream_put_le32(&cropping, track->video.pixel_cropr); >> + } >> if (par->codec_id != AV_CODEC_ID_HEVC) >> sti->need_parsing = AVSTREAM_PARSE_HEADERS; >> >> @@ -3136,10 +3158,21 @@ static int matroska_parse_tracks(AVFormatContext *s) >> track->default_duration = default_duration; >> } >> } >> + track->video.cropped_width = track->video.pixel_width; >> + track->video.cropped_height = track->video.pixel_height; >> + if (track->video.display_unit == 0) { >> + if (track->video.pixel_cropl >= INT_MAX - track->video.pixel_cropr || >> + track->video.pixel_cropt >= INT_MAX - track->video.pixel_cropb || >> + (track->video.pixel_cropl + track->video.pixel_cropr) >= track->video.pixel_width || >> + (track->video.pixel_cropt + track->video.pixel_cropb) >= track->video.pixel_height) >> + return AVERROR_INVALIDDATA; >> + track->video.cropped_width -= track->video.pixel_cropl + track->video.pixel_cropr; >> + track->video.cropped_height -= track->video.pixel_cropt + track->video.pixel_cropb; >> + } >> if (track->video.display_width == -1) >> - track->video.display_width = track->video.pixel_width; >> + track->video.display_width = track->video.cropped_width; >> if (track->video.display_height == -1) >> - track->video.display_height = track->video.pixel_height; >> + track->video.display_height = track->video.cropped_height; >> } else if (track->type == MATROSKA_TRACK_TYPE_AUDIO) { >> if (!track->audio.out_samplerate) >> track->audio.out_samplerate = track->audio.samplerate; >> -- >> 2.45.1 > > Have you seen the discussion over the MKVToolNix issue tracker? > https://gitlab.com/mbunkus/mkvtoolnix/-/issues/2389 > > I think it is the reason it wasn't implemented in ffmpeg sooner. The > issue summarizes the problem and current status quo, quite well I > think. I think the main problem is that vast majority of Matroska > files doesn't are not following the standard with regards to > DisplayWidth/Height. The only thing i can think to do about this is adding an option to ignore display dimension fields and have the demuxer derive them as if they were not coded in. There's no amount of heuristics one could do to figure out if the values in display dimension fields are correct and intended (taking into account any cropping value that may be present, and/or trying to achieve a certain DAR), or if they are bogus. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".