From patchwork Sat Sep 7 00:00:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Niki Bowe X-Patchwork-Id: 14964 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 3EE7E44A612 for ; Sat, 7 Sep 2019 03:08:24 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 181FB687FB9; Sat, 7 Sep 2019 03:08:24 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A41F5687FAE for ; Sat, 7 Sep 2019 03:08:17 +0300 (EEST) Received: by mail-wm1-f66.google.com with SMTP id n10so8811952wmj.0 for ; Fri, 06 Sep 2019 17:08:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=BeHdoIzUVY0yVOilr4jKN3rQbvMYDBEKN2k9W7lPMZg=; b=t/rFILh6igo19a+6R2Sn1i07A2hJx7yEs42fNHld9gvsavwAsEK779emSc1PplA1VN LzMIjLVJ2avDR4thh/L7OYk+VF2SGlSMUJjT/ubbGIJSAt7EDjhM2nLOqKzPMX7WUa6r z9xhy307AxTyRbwTxZr2s/WeHpkgbwCU4gkNcVsLqO57/Et8l+3Opt3noFfZyMLXE4D/ XqU9ghpkC+yT1H9QkfFHmySRobZQ548CdA67kpmQcNd2r9xrvKwurat9aU8EVayYlPq3 LJEsLtL8M3ZULJVPw+0BdvPkYu0VqRqut7WEuz2b2cQDAh1IHBxADv+DAh052e9cLDhW TbVA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=BeHdoIzUVY0yVOilr4jKN3rQbvMYDBEKN2k9W7lPMZg=; b=I1lMmIysZL3TrmnD6QXExJ8JEVyA1D4nVI9uu0DesynE0/ztVZBmtkg211Yg54/2xe 1gg3FQHobsu6Itp71iCOGXPOgXh7shFvuui5GSTmITNVQhWdmaNcTJxyKp+EbybKGrv8 qPerQY++t/kx4QQl6C1oyjLixnS6HMcFG68FtXyu17pcWjLge0fw+tUYbE8fKHDA12QN dqtSqsw5jDtfHlh5gf5BhKlm1af+hmNNEumbiDKgWJYRpWvWZKgqAiFz8V8sM/zAnXXI 9CzVbTqttCEc1Tf6KzYzUTWrIczZoBIqvDa+W1hy8JGQO4xbHajMQYnoUCPx0QgMTZ37 Ss+A== X-Gm-Message-State: APjAAAXhmsDJKnteVeaJweNDOFHwFbhlJdTJnQ0eUQxBbJAEiDbr47hd dVi9qcbYqoUIKrHZSoCP3ux0ldJZpwRsnXi4RuPShM1cxxQ= X-Google-Smtp-Source: APXvYqywknOIyU3Xjho5Eg2GfTzTfhNKJccZjHUI7n1l3C7TicdU7u5AChSmBRY73Wdh9JJpY4l0zI/UBhbastWUy/I= X-Received: by 2002:a1c:9a4a:: with SMTP id c71mr8743670wme.99.1567814427480; Fri, 06 Sep 2019 17:00:27 -0700 (PDT) MIME-Version: 1.0 References: <20190820013648.161805-1-nbowe@google.com> <20190825183939.GF3219@michaelspb> In-Reply-To: From: Niki Bowe Date: Fri, 6 Sep 2019 17:00:14 -0700 Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.20 Subject: Re: [FFmpeg-devel] [PATCH] avformat: Fix probing on some JPEGs X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" As it turns out the last patch still lets a lot of jpegs get misidentified. This new version avoids the problem by checking for jpeg magic at the start. I also added a FATE test, and attached the jpeg for the test. On Tue, Aug 27, 2019 at 6:30 PM Niki Bowe wrote: > > > On Sun, Aug 25, 2019 at 11:39 AM Michael Niedermayer > wrote: > >> On Fri, Aug 23, 2019 at 04:03:10PM -0700, Niki Bowe wrote: >> > On Thu, Aug 22, 2019 at 2:30 AM Paul B Mahol wrote: >> > >> > > On Thu, Aug 22, 2019 at 11:19 AM Carl Eugen Hoyos > > >> > > wrote: >> > > >> > > > Am Mi., 21. Aug. 2019 um 23:05 Uhr schrieb Niki Bowe >> > > > : >> > > > > >> > > > > On Mon, Aug 19, 2019 at 7:22 PM Carl Eugen Hoyos < >> ceffmpeg@gmail.com> >> > > > wrote: >> > > > > >> > > > > > >> > > > > > This score would mean that mjpeg can never be detected. >> > > > > > I suspect you have to reduce one of the demuxers to "- 1". >> > > > > > >> > > > > > >> > > > > Thanks Carl. >> > > > > Attached patch to reduce mpeg probe by -1, which also fixes the >> issue. >> > > > >> > > > Sorry, I misread the original report, it looked to me as if mJpeg >> was >> > > > the culprit. >> > > > >> > > > Imo, the mpeg probing should be fixed (return a smaller value) for >> your >> > > > sample >> > > > by detecting that it is not mpeg, not by returning a smaller value >> for >> > > > all samples. >> > > > >> > > >> > > 1000% agree. >> > > >> > > >> > It didn't return a smaller value for all samples, only the "invalid VDR >> > files and short PES streams" case. >> > Most mpegps files still return 26 immediately, because they have pack >> > headers. >> > >> > However, here is another patch where I try to limit it to only changing >> > score for these jpegs. >> > I noticed that these jpegs had a lot of 0x00000100 sequences, which >> matches >> > mpeg picture header start code. I added another heuristic which matches >> > these jpegs, but not any mpegps files I could find. >> > >> > Alternatively I could make reduce score if it doesn't start with a start >> > code? At the moment its happy to search until it finds start codes. >> > >> > >> > Is everyone really sure the best approach is to modify mpegps_probe for >> > this? >> > The mpegps_probe function returns 25 in many instances where it may not >> be >> > mpegps. It does only minimal structural checking, and allows invalid >> data >> > to still classify as mpegps. >> > jpeg probing returns 25 in some cases where it is almost certainly a >> jpeg >> > (Has to go through multiple tags to get to SOS, many of which early out >> if >> > they find invalid data). >> > Note that 25 is still treated as "low confidence" for jpeg. It logs >> "Format >> > jpeg_pipe detected only with low score of 25, misdetection possible!" >> for >> > these jpegs. >> > So I still think a score of 25 is too low for these jpegs, and that a >> > better fix would be to return 26 for jpeg_pipe and mjpeg if it makes it >> > past multiple tags to SOS. >> >> jpegs can be in other container formats its not jpeg in that case but the >> other container format >> > about this patch >> it breaks this: >> >> ./ffplay tickets//3327/issue3327-libc-2.17.so >> >> https://trac.ffmpeg.org/raw-attachment/ticket/3327/issue3327-libc-2.17.so >> >> which is detected as mpeg after the patch. >> really nothing should be detected as mpeg after this that was not before >> the idea IIUC is make something that was detected as mpeg to be not >> anymore >> >> > Thanks Michael. Good find. > Attached patch which only applies the extra sanity checks to the "Invalid > VDR files and short PES streams" path. > > > >> Thanks >> >> [...] >> >> >> -- >> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB >> >> Awnsering whenever a program halts or runs forever is >> On a turing machine, in general impossible (turings halting problem). >> On any real computer, always possible as a real computer has a finite >> number >> of states N, and will either halt in less than N cycles or never halt. >> _______________________________________________ >> 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". > > > > -- > > Nikolas Bowe | SWE | nbowe@google.com | 408-565-5137 > From 49fdc6549a0b8413f80aa16a14473e956d0ea08b Mon Sep 17 00:00:00 2001 From: Nikolas Bowe Date: Wed, 4 Sep 2019 18:23:16 -0700 Subject: [PATCH] Fix JPEGs being misidentified as mpeg. mpeg probing is very forgiving, willing to skip over arbitrary junk between "start codes". Its not unusual to find things which look like start codes in other formats. Normally that isn't a problem as other formats will get higher probe scores, but jpeg probe scores will only be 25 unless it reaches the End Of Image tag. For this reason, if it starts with JPEG magic, treat it as not an mpeg. --- libavformat/mpeg.c | 5 +++++ tests/fate/probe.mak | 3 +++ 2 files changed, 8 insertions(+) diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c index 3205f209e6..d8657c397e 100644 --- a/libavformat/mpeg.c +++ b/libavformat/mpeg.c @@ -96,6 +96,11 @@ static int mpegps_probe(const AVProbeData *p) else if ((code & 0xf0) == VIDEO_ID && !pes) invalid++; else if ((code & 0xe0) == AUDIO_ID && !pes) invalid++; else if (code == PRIVATE_STREAM_1 && !pes) invalid++; + } else if (i == 3) { + if ((code & 0xFFFFFF00) == 0xFFD8FF00) { + // Starts with JPEG magic. + return 0; + } } } diff --git a/tests/fate/probe.mak b/tests/fate/probe.mak index 4be9356f5f..8114af48a5 100644 --- a/tests/fate/probe.mak +++ b/tests/fate/probe.mak @@ -7,6 +7,9 @@ fate-probe-format-roundup1383: REF = mp3 FATE_PROBE_FORMAT-$(CONFIG_MPEGPS_DEMUXER) += fate-probe-format-roundup1414 fate-probe-format-roundup1414: REF = mpeg +FATE_PROBE_FORMAT-$(call ALLYES, MPEGPS_DEMUXER IMAGE2_DEMUXER MJPEG_DECODER) += fate-probe-format-jpeg_with_mpeg_start_codes +fate-probe-format-jpeg_with_mpeg_start_codes: REF = jpeg_pipe + FATE_PROBE_FORMAT-$(CONFIG_DV_DEMUXER) += fate-probe-format-roundup2015 fate-probe-format-roundup2015: REF = dv -- 2.23.0.187.g17f5b7556c-goog