[FFmpeg-devel] [PATCH v3 1/1] fftools/ffprobe: print size of attachment streams (extradata_size)

Sat Nov 27 16:12:33 EET 2021

On Sat, Nov 27, 2021 at 01:12:31PM +0000, Soft Works wrote:
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Michael
> > Niedermayer
> > Sent: Saturday, November 27, 2021 1:19 PM
> > To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH v3 1/1] fftools/ffprobe: print size of
> > attachment streams (extradata_size)
> > 
> > On Sat, Nov 27, 2021 at 12:58:30AM +0100, Michael Niedermayer wrote:
> > > On Fri, Nov 26, 2021 at 10:01:23PM +0000, Soft Works wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: ffmpeg-devel <mailto:ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > Michael
> > > > > Niedermayer
> > > > > Sent: Friday, November 26, 2021 10:26 PM
> > > > > To: FFmpeg development discussions and patches <mailto:ffmpeg-devel at ffmpeg.org>
> > > > > Subject: Re: [FFmpeg-devel] [PATCH v3 1/1] fftools/ffprobe: print size
> > of
> > > > > attachment streams (extradata_size)
> > > > >
> > > > > On Fri, Nov 26, 2021 at 07:55:40PM +0000, Soft Works wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: ffmpeg-devel <mailto:ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > Michael
> > > > > > > Niedermayer
> > > > > > > Sent: Friday, November 26, 2021 7:50 PM
> > > > > > > To: FFmpeg development discussions and patches <mailto:ffmpeg-devel at ffmpeg.org>
> > > > > > > Subject: Re: [FFmpeg-devel] [PATCH v3 1/1] fftools/ffprobe: print
> > size of
> > > > > > > attachment streams (extradata_size)
> > > > > > >
> > > > > > > On Thu, Nov 25, 2021 at 04:59:41PM +0000, Soft Works wrote:
> > > > > > > > Another attempt: Created on Linux and zipped...
> > > > > > >
> > > > > > > tested and works
> > > > > > > LGTM
> > > > > >
> > > > > > Thanks.
> > > > > >
> > > > > > Today I've seen that the v3 version with the patch as an attachment
> > has
> > > > > > been processed by patchwork:
> > > > > >
> > > > > >
> > > > >
> > https://patchwork.ffmpeg.org/project/ffmpeg/patch/CH0P223MB03639F30A548FA85C1
> > > > > mailto:7E8855BA629 at CH0P223MB0363.NAMP223.PROD.OUTLOOK.COM/
> > > > > >
> > > > > > I've also verified that the output from git format-patch on Linux was
> > the
> > > > > > same as on Windows. Also, I sent e-mails with the attached patch to
> > myself
> > > > > > (google-to-google and google-to-ms) and verified that the attachments
> > were
> > > > > > unchanged.
> > > > > >
> > > > > > My preliminary conclusion for patches containing long lines:
> > > > > >
> > > > > > For patchwork: Use the --attach param for git send-patch
> > > > > > For Michael and maybe others: send as zip attachment 😊
> > > > > > (not that I could tell the reason)
> > > > > >
> > > > > > Might be interesting to check whether there's a difference between
> > the
> > > > > > e-mail you get and the file you get when downloading the mbox from
> > > > > patchwork.
> > > > >
> > > > > The attachemnt after the mailing list is broken:
> > > > > https://ffmpeg.org/pipermail/ffmpeg-
> > > > > devel/attachments/20211125/daa80b4e/attachment.bin
> > > > >
> > > > > if you want to help fix this, thats welcome
> > > > >
> > > > > That patchwork has a working patch is interresting but iam not sure if
> > the
> > > > > awnser to this will help, the problem seems before. So we need a fix
> > before
> > > > > if you are interrested in helping, there was some prior occurance of
> > this
> > > > > maybe there was some additional information in that thread
> > > >
> > > > There are two problems - I'm not sure which one you are talking about:
> > >
> > > theres a problem which has occured previously, for which there is one
> > > or more threads on the ML
> > > if you want to help, find these threads. Throwing out ideas which are
> > > not even consistent with the current facts causes people like me to
> > > waste time looking at the wrong places.
> > > Let alone the problem was not that we didnt know where the bug was
> > > IIRC it was that someone had to fix it
> > > It is true that the new instance here has some currently unexplained
> > > behaviours but neither your 1. or 2. works at explaining them
> > >
> > >
> > > [...]
> > >
> > > > 2. You didn't correctly receive the patch sent as attachment
> > > >
> > > > I just double-checked: The ML software didn't modify the patch when
> > attached
> > > > (as plain-text .patch)
> > >
> > > how did you check that ?
> > > it is broken on my side and it is broken on the server as you can see by
> > > looking at the link i posted
> > > Also i mailed the correct patch to myself as plain/text attachment and
> > > its perfectly fine. Neither my SMTP server not my local mail handling
> > > did anything odd to the long line
> > > OTOH the mail i received from the ML though contains a base64 encoded
> > > attachment and inside that is a broken patch
> > > if you say you have received a good one then my first intuition is to
> > > ask you to check that again
> > > i took the attachment decoded it with command line base64 -d and it
> > > was not good
> > >
> > >
> > > >
> > > > Patchwork could apply this correctly, so I'm not sure what went wrong
> > > > in that case when you couldn't apply it?
> > >
> > > Your attachment is broken on the server already, again
> > > your mail with the attachment:
> > > https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-November/288260.html
> > > and the attachment:
> > > https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20211125/daa80b4e/attachment.bin
> > > a954c4f6237453e91ab3e0327ec5eae9  attachment.bin <-- this is the broken one
> > >
> > > patch --dry-run -p1 <attachment.bin
> > > checking file doc/ffprobe.xsd
> > > checking file fftools/ffprobe.c
> > > Hunk #1 succeeded at 2769 (offset -14 lines).
> > > checking file tests/ref/fate/concat-demuxer-extended-lavf-mxf
> > > checking file tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
> > > checking file tests/ref/fate/concat-demuxer-simple1-lavf-mxf
> > > checking file tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
> > > checking file tests/ref/fate/concat-demuxer-simple2-lavf-ts
> > > checking file tests/ref/fate/flv-demux
> > > checking file tests/ref/fate/mov-zombie
> > > checking file tests/ref/fate/mxf-probe-d10
> > > checking file tests/ref/fate/oggopus-demux
> > > checking file tests/ref/fate/ts-demux
> > > checking file tests/ref/fate/ts-opus-demux
> > > checking file tests/ref/fate/ts-small-demux
> > > patch unexpectedly ends in middle of line
> > > Hunk #1 FAILED at 145.
> > > 1 out of 1 hunk FAILED
> > >
> > > you can just try to download it from the web archieve and
> > > try for yourself its broken before my mail stuff ever sees it
> > > and that is not inline
> > > Neither your case 1. nor 2. explain this and this is a bug
> > > on the server somewhere, i dont remember where but i think i
> > > knew it previosuly what caused this
> > > something along the postfix - mailman chain probably
> > > i also faintly remeber considering to simply bump the 998
> > > limit up to workaround it
> > > maybe anton or someone else remembers more details
> > > I sadly dont have enough time ATM to really re-investigate this
> > 
> > I had a bit more time to look into this
> > 
> > this issue may be unrelated to the 998 line limit, the patch on this mail
> > is simply corrupted
> > no matter if i take it from the received mail or the mailman archive
> > 
> > patch --dry-run -p1 <~/v3-0001-fftools-ffprobe-print-size-of-attachment-
> > streams-.patch
> > ...
> > checking file tests/ref/fate/ts-small-demux
> > patch unexpectedly ends in middle of line
> > Hunk #1 FAILED at 145.
> > 1 out of 1 hunk FAILED
> > 
> > but with -F9
> > patch --dry-run -p1 -F9  <~/v3-0001-fftools-ffprobe-print-size-of-attachment-
> > streams-.patch
> > checking file tests/ref/fate/ts-small-demux
> > patch unexpectedly ends in middle of line
> > Hunk #1 succeeded at 145 with fuzz 3.
> > 
> > This 2nd case is likely how patchwork could apply the patch
> > 
> > the corruption is a missing crlf, if i add it by hand it works without -F9
> > 
> > if i apply it locally and rebuild the patch with format-patch
> > 
> > git format-patch -1
> > git checkout HEAD^
> > patch --dry-run -p1 <0001-Try.patch
> > it works fine
> 
> 
> That’s a really tricky case, because v2 of the patch (sent inline) had 
> in fact incorrect line breaks (see screenshot).
> 
> Then I sent the patch as an attachment (v3) and that in turn had the effect that
> the last line is ending without a line break.
> 
> Finally I sent the whole patch as a zipped attachment - which worked (of course),
> and the patch you created locally works as well (not surprising).
> 
> But there are still two problems:
> 

> 1. Patches sent to the ML as inline get long lines truncated

This should not be surprising, the SMTP RFC recommands this truncation
maybe this could be avoided if either mailman or postfix encode the message
as base64 either always or conditional on long lines
if someone who knows how to do this reads this, please explain. But keep in
mind maintaining code changes in mailman is something i want to avoid so
future security updates stay easy
Also i assume that inline mail wasnt base64 when it was received, i didnt 
check, if it already was then i guess that wouldnt help
The alternative is to make postfix not do this truncation and ignore the RFC
that is easy. But do we want that ? (again i assume here it was postfix
which did this truncation i did not confirm this)

> 2. Patches sent as attachment are missing a final line break
> 
> for 1., I'm sure it's a universal problem for everybody.

> for 2., I'm wondering whether it's just my E-Mail client which removes that last 
> line break when sending or whether that happens somewhere else...

as i apply alot of patches and this isnt normally happening the question arrises
what was different here. It was a patch attached by you. and it was a
patch that had some pretty long lines, maybe both these are unrelated i dont know

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Homeopathy is like voting while filling the ballot out with transparent ink.
Sometimes the outcome one wanted occurs. Rarely its worse than filling out
a ballot properly.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20211127/65ca1d22/attachment.sig>