diff mbox series

[FFmpeg-devel] lavf/imx: Do not probe number of frames

Message ID CAB0OVGpv56ip4Z4E1wXPVWKvoeMwJGmjn4_xkouchF8d-=hdsA@mail.gmail.com
State New
Headers show
Series [FFmpeg-devel] lavf/imx: Do not probe number of frames | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Carl Eugen Hoyos Feb. 21, 2021, 9:43 a.m. UTC
Hi!

The Simbiosis IMX probe function looks strict enough, do not check for
a field that does not cause decoding to fail.

Please comment, Carl Eugen
Subject: [PATCH] lavf/imx: Do not probe number of frames.

The field never causes decoding to fail.
---
 libavformat/imx.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Paul B Mahol Feb. 21, 2021, 10:39 a.m. UTC | #1
This patch is not good.

Probe function should not be changed.
Carl Eugen Hoyos Feb. 21, 2021, 11:20 a.m. UTC | #2
Am So., 21. Feb. 2021 um 11:39 Uhr schrieb Paul B Mahol <onemda@gmail.com>:
>
> This patch is not good.

Could you elaborate?

Carl Eugen
Paul B Mahol Feb. 21, 2021, 11:35 a.m. UTC | #3
Because there is not point in it.
Number of frames should always be probed.
Carl Eugen Hoyos Feb. 21, 2021, 1:02 p.m. UTC | #4
Am So., 21. Feb. 2021 um 12:35 Uhr schrieb Paul B Mahol <onemda@gmail.com>:
>
> Because there is not point in it.
> Number of frames should always be probed.

It is neither necessary nor useful to probe it.
Both is immediately obvious:
You already check 48 bits, ruling out one possible value
for 32bit does not improve this detection.
The value is not needed for correct decoding.

Carl Eugen
Paul B Mahol Feb. 21, 2021, 1:12 p.m. UTC | #5
On Sun, Feb 21, 2021 at 2:10 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> Am So., 21. Feb. 2021 um 12:35 Uhr schrieb Paul B Mahol <onemda@gmail.com
> >:
> >
> > Because there is not point in it.
> > Number of frames should always be probed.
>
> It is neither necessary nor useful to probe it.
> Both is immediately obvious:
> You already check 48 bits, ruling out one possible value
> for 32bit does not improve this detection.
> The value is not needed for correct decoding.
>

NAK, This is final decision. Give up.

Make something more useful.



>
> Carl Eugen
> _______________________________________________
> 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".
Carl Eugen Hoyos Feb. 21, 2021, 1:22 p.m. UTC | #6
Am So., 21. Feb. 2021 um 14:12 Uhr schrieb Paul B Mahol <onemda@gmail.com>:
>
> On Sun, Feb 21, 2021 at 2:10 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> > Am So., 21. Feb. 2021 um 12:35 Uhr schrieb Paul B Mahol <onemda@gmail.com
> > >:
> > >
> > > Because there is not point in it.
> > > Number of frames should always be probed.
> >
> > It is neither necessary nor useful to probe it.
> > Both is immediately obvious:
> > You already check 48 bits, ruling out one possible value
> > for 32bit does not improve this detection.
> > The value is not needed for correct decoding.
>
> NAK, This is final decision. Give up.

> Make something more useful.

I do consider this useful:
Nobody will ever look at this code again except
if another developer uses it as blueprint for a
bad probe function.
And that is apart from the question how unlikely
it is that a game exists where the imx files do not
store the number of frames.

Why can't you concentrate on things I know
nothing about and let me do the boring stuff?

Carl Eugen
Paul B Mahol Feb. 21, 2021, 1:50 p.m. UTC | #7
On Sun, Feb 21, 2021 at 2:23 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> Am So., 21. Feb. 2021 um 14:12 Uhr schrieb Paul B Mahol <onemda@gmail.com
> >:
> >
> > On Sun, Feb 21, 2021 at 2:10 PM Carl Eugen Hoyos <ceffmpeg@gmail.com>
> wrote:
> >
> > > Am So., 21. Feb. 2021 um 12:35 Uhr schrieb Paul B Mahol <
> onemda@gmail.com
> > > >:
> > > >
> > > > Because there is not point in it.
> > > > Number of frames should always be probed.
> > >
> > > It is neither necessary nor useful to probe it.
> > > Both is immediately obvious:
> > > You already check 48 bits, ruling out one possible value
> > > for 32bit does not improve this detection.
> > > The value is not needed for correct decoding.
> >
> > NAK, This is final decision. Give up.
>
> > Make something more useful.
>
> I do consider this useful:
> Nobody will ever look at this code again except
> if another developer uses it as blueprint for a
> bad probe function.
> And that is apart from the question how unlikely
> it is that a game exists where the imx files do not
> store the number of frames.
>
> Why can't you concentrate on things I know
> nothing about and let me do the boring stuff?
>

This is wrong stuff to do.

Better remove not needed header  above.

>
> Carl Eugen
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavformat/imx.c b/libavformat/imx.c
index 66f18177a5..bcf3a16d38 100644
--- a/libavformat/imx.c
+++ b/libavformat/imx.c
@@ -38,8 +38,6 @@  static int simbiosis_imx_probe(const AVProbeData *p)
 {
     if (AV_RL32(p->buf) != IMX_TAG)
         return 0;
-    if (AV_RN32(p->buf+4) == 0)
-        return 0;
     if (AV_RN16(p->buf+8) == 0)
         return 0;
     if (AV_RL16(p->buf+10) != 0x102)