[FFmpeg-devel-irc] IRC log for 2010-05-09

[irc at mansr.com]

[00:17:57] * Compn trolls on Dark_Shikari's blog
[00:18:24] <CIA-7> libswscale: mru * r31142 /trunk/libswscale/bfin/internal_bfin.S:
[00:18:24] <CIA-7> libswscale: blackfin: fix yuv422 to yuv420 conversion
[00:18:24] <CIA-7> libswscale: The old code is correct only when stride = 2*width.
[00:18:24] <CIA-7> libswscale: Patch by Ronaldo Moura <ronaldo d moura monity com br>
[00:18:58] <mru> who's "louise"?
[00:20:55] <Compn> ?
[00:21:07] <mru> commenter on Dark_Shikari's blog
[00:21:23] <mru> is that your trolling alias?
[00:21:52] <Compn> no
[00:22:00] <Dark_Shikari> he trolled a previous post posting tons of shit about vp8 was amazing
[00:22:04] <Dark_Shikari> and google must have put their best engineers on it
[00:22:05] <Dark_Shikari> etc etc
[00:22:09] <Dark_Shikari> and the post wasn't even remotely related to vp8
[00:22:13] <Dark_Shikari> so I just deleted it all
[00:22:44] <Compn> i wonder how high ffmpeg devel posts rank on vp8 news
[00:23:05] <mru> ffmpeg devs are ignored at large
[00:23:18] <mru> except when monty gets pissed
[00:24:17] <mru> wtf is jmkta?
[00:24:20] <mru> and hhi?
[00:24:41] <Dark_Shikari> JM-KTA is the baseline test platform
[00:24:43] <Dark_Shikari> used for comparison
[00:24:51] <Dark_Shikari> it's a modified version of JM with various post-H.264 features added
[00:28:03] <mru> Dark_Shikari: btw, your blog colour scheme isn't very nice
[00:28:19] <Dark_Shikari> is the background not dark enough?
[00:28:37] <mru> but don't worry, I totally restyled it with Stylish
[00:28:55] <mru> black text, light background, serif font
[00:29:18] <Dark_Shikari> that is called "burning my eyes"
[00:29:36] <Dark_Shikari> white background == staring at a bright light
[00:29:46] <mru> well, light blue on navy background isn't exactly friendly
[00:29:57] <Dark_Shikari> it's extremely light gray on dark navy
[00:29:59] <Dark_Shikari> aka white on black
[00:30:13] <Dark_Shikari> oh, also, just noticed a rather silly UI bug with ffmpeg
[00:30:17] <Dark_Shikari> if you try to open a file that doesn't exist
[00:30:17] <Dark_Shikari> it says
[00:30:18] <Dark_Shikari> Error number -2 occurred
[00:30:21] <mru> well, I prefer black on light grey
[00:30:30] <mru> hehe
[00:31:28] <drv> mine says "no such file or directory"
[00:31:32] <drv> are you on windows perhaps?
[00:31:44] <Dark_Shikari> yes
[00:31:52] <Dark_Shikari> mingw build
[00:32:02] <drv> i think some of the error -> string stuff was changed to use strerror
[00:32:09] <Dark_Shikari> and mingw's strerror sucks probably
[00:32:10] <drv> which probably is a stub/missing on mingw
[00:32:54] <mru> s/strerror//
[00:33:06] <drv> heh
[00:37:01] <drv> hm, actually the MS CRT implements strerror, so maybe there is something else going on
[00:37:16] <drv> i don't think it has strerror_r, that's probably it
[00:39:18] <drv> looks like they have their own incompatible "safe" strerror_s()
[00:52:35] <drv> ... which, for some reason, is not declared in the w32api headers
[04:55:09] <ramiro> drv: try using mingw-w64. I'm starting to use it even for my 32-bit builds. (there's a catch though, you have to pass --extra-cflags=-Dstrtor=__strtod).
[07:27:44] <siretart> ramiro: did win64 work in ffmpeg 0.5?
[07:53:15] <Dark_Shikari> it never worked
[10:26:53] <BastyCDGS> mru, are u here?
[10:27:16] <mru> I am always here, and everywhere else
[10:27:20] <mru> I am the omnitroll
[10:27:40] <BastyCDGS> I was just looking again on the source line you've been looking yesterday evening in TuComposer
[10:28:24] <BastyCDGS> where you wondered about that UWORD cast
[10:28:30] <mru> on second thoughts, that cast is safe
[10:28:34] <mru> but unnecessary
[10:28:42] <BastyCDGS> it's reading from a byte
[10:28:45] <BastyCDGS> signed byte
[10:28:55] <BastyCDGS> but sign extending wasn't necessary beause of << 8
[10:28:59] <mru> yes, due to the range of values involved it's safe
[10:29:07] <mru> so why use signed byte?
[10:29:10] <BastyCDGS> this cast skips creating an ext.w instruction
[10:29:12] <mru> and not unsigned?
[10:29:15] <BastyCDGS> because samples are signed ;)
[10:29:26] <mru> doesn't matter
[10:29:34] <mru> your manipulating the value as unsigned
[10:29:40] <mru> you're
[10:30:04] <BastyCDGS> yes in that case I do it and that's why I use the cast
[10:30:09] <mru> what matters is not what it is, but what you do with it
[10:30:11] <BastyCDGS> it really creates faster code here on m68k
[10:30:27] <mru> faster than declaring the pointer of the right type to begin with?
[10:31:10] <BastyCDGS> samples are mostly processed really as signed
[10:31:25] <BastyCDGS> this routine was an exception where a small part of it is really faster with threating it at unsigned
[10:31:51] <mru> so declare the pointer that way there
[10:33:20] <BastyCDGS> btw, what have you meant with undefined behaviour you said you were opening a random file and saw it?
[10:33:41] <mru> conversion from unsigned to signed is potentially undefined
[10:33:59] <mru> but in this case the value is always in the defined range
[10:35:51] <BastyCDGS> you mean you can't be sure if a compiler decides to strip to MAX_INT if you converted a too large int to uint?
[10:36:26] <mru> the C standard says conversion to signed int from a value out of range is undefined
[10:36:40] <Vitor1001> BastyCDGS: do you read -cvslogs ML?
[10:37:08] <BastyCDGS> no I don't Vitor, why?
[10:37:15] <mru> you should
[10:37:27] <Vitor1001> http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/2010-May/029380.html
[10:39:12] <mru> I think I see the problem
[10:39:17] <mru> --x instead of x--
[10:40:40] <mru> yep
[10:41:09] <mru> btw, that for loop is bad style
[10:41:10] <BastyCDGS> oh there are problems
[10:41:18] <mru> sorry I missed that in the review
[10:42:55] <BastyCDGS> I see there are missing 8 bytes at the end
[10:43:14] <BastyCDGS> so it's the loop just counting one to less
[10:43:29] <mru> that's very bad style
[10:43:45] <mru> avoid side-effects in the test expression of a for loop
[10:44:34] <BastyCDGS> I did it because I saw it's 200 cycles faster because it doesn't have to compare the pointer (i.e. wait for add)
[10:44:49] <BastyCDGS> question is if we change to x-- if it's still faster then
[10:45:00] <mru> oh lord...
[10:45:01] <BastyCDGS> maybe it's better to increment x by one before start of loop
[10:45:05] <mru> NO
[10:45:12] <mru> it doesn't fucking matter
[10:45:23] <mru> the compiler will do the right thing
[10:45:33] <mru> this is one of the rare things compilers do right
[10:46:17] <BastyCDGS> should I do a fix now?
[10:46:26] <BastyCDGS> or are u already doing this?
[10:46:30] <mru> I'm sending an email
[10:50:35] <BastyCDGS> just read your while loop patch
[10:50:42] <BastyCDGS> what about using do ... while instead?
[10:50:47] <mru> why?
[10:51:05] <mru> the compiler will do the very same thing
[10:51:15] <mru> it will use a decrement-and-test instruction of some sort
[10:51:22] <mru> assuming the cpu has one
[10:51:50] <BastyCDGS> I meant the initial jump into the loop, at least for does it, it makes a jump at the end of for loop and checks if the condition is true
[10:52:13] <mru> you should spend more time disassembling compiled code
[10:52:17] <BastyCDGS> since pixel width is rounded to nearest word (i.e. is at least 2 bytes) it should be ok
[10:52:45] <mru> that's irrelevant
[10:53:08] <mru> while (x--) { } and do { } while (--x) are mostly equivalent
[10:53:13] <mru> when the initial value is non-zero
[10:54:53] <BastyCDGS> yes that's why it's relevant that I stated that it's at least 2 bytes (and can therefore not initially be zero)
[10:55:15] <mru> at least one would be good enough
[10:55:25] <mru> and a zero-width image is invalid
[10:55:56] <mru> you need to stop thinking in these terms
[10:56:02] <mru> branches are fast nowadays
[10:56:09] <mru> we have branch prediction
[10:56:32] <mru> and compilers are pretty good at transforming loop control expressions to be efficient
[10:56:55] <mru> at least when the loop control variables are not used within the loop
[10:57:09] <BastyCDGS> btw, what is 10l?
[10:57:14] <BastyCDGS> lol?
[10:57:17] <mru> 10 liters
[10:57:22] <mru> of something you don't like
[10:57:37] <mru> it's the punishment for a silly mistake
[10:57:38] * kierank pretends not to like beer
[10:57:50] <mru> kierank: it doesn't work if you pretend
[10:58:04] <mru> kierank: btw, aren't you british?
[10:58:41] <kierank> yes. you have basty a 10l of something he doesn't like so when i get mine i'll pretend not to like beer
[10:58:44] <kierank> gave*
[10:58:56] <BastyCDGS> :D
[10:59:06] <mru> pretending not to like beer is impossible for a brit
[10:59:30] <kierank> that is true
[10:59:32] <mru> unless it's that ghastly french "beer"
[10:59:38] <kierank> or fosters
[10:59:46] <mru> but then those are not beer
[11:00:01] <mru> so not liking those isn't not liking beer
[11:31:12] <Kovensky> <@mru> this is one of the rare things compilers do right <-- I didn't know they did stuff right :o
[11:57:30] <BastyCDGS> oh error
[11:57:38] <BastyCDGS> when I configure --disable-avfilter --disable-swscale I get:
[11:58:09] <BastyCDGS> on linking: /home/basty/src/ffmpeg/cmdutils.c:614: undefined reference to sws_isSupportedOutput
[11:58:09] <BastyCDGS> and /home/basty/src/ffmpeg/cmdutils.c:614: undefined reference to sws_isSupportedInput
[12:03:42] <BastyCDGS> after fixing it manually I noticed it also won't build ffplay anymore
[12:04:55] <mru> of course
[12:04:58] <mru> ffplay needs it
[12:05:38] <CIA-7> ffmpeg: mru * r23062 /trunk/cmdutils.c: Fix build with swscale disabled
[12:05:59] <BastyCDGS> btw, is avfilter now enabled by default?
[12:06:08] <BastyCDGS> noticed this because iff crashes with avfilter enabled
[12:06:20] <BastyCDGS> but that's unrelated to my changes (the very old IFF crashed, too)
[12:13:05] <_av500_> mru: ill update the patch for the other 2 bits tomorrow..
[12:13:17] <mru> are they needed?
[12:13:27] <mru> those functions are always built
[12:13:57] <_av500_> i have them in my private tree
[12:14:05] <BastyCDGS> mru, I just noticed that the IFF demuxer doesn't check that width & height are != 0 actually
[12:14:13] <_av500_> at least the bsf one i had to add, it would not build
[12:14:39] <mru> that's odd
[12:14:56] <mru> av_bitstream_filter_next() is in bitstream_filter.c
[12:15:02] <mru> and that's built unconditionally
[12:15:09] <_av500_> hmm
[12:15:11] <mru> well, not if avcodec is disabled
[12:15:14] <mru> but then nothing is built
[12:15:16] <_av500_> i can check again tomorrow
[12:34:24] <BastyCDGS> updated HAM patch submitted to ml
[12:34:39] <BastyCDGS> mru, it seems you have forgotten to commit the bugfix while patch to git
[12:40:06] <mru> no, I didn't forget
[12:41:13] <BastyCDGS> are you waiting for reviews?
[13:40:05] <mru> it's not my file, so I'm supposed to send a patch first
[13:40:17] <mru> if nobody replies I'll probably just commit it anyway
[13:41:53] <BastyCDGS> I just can confirm that it works with your patch ;)
[13:42:04] <mru> I already checked that
[14:38:38] <ramiro> siretart: what Dark_Shikari said. it's not a regression, it never fully worked.
[15:21:34] <BastyCDGS> has someone investigated why ffplay crashes with --enable-avfilter configure with IFF stuff?
[15:23:51] <Vitor1001> BastyCDGS: open a roundup ticket
[15:24:20] <BastyCDGS> it was opened already quite a long time
[15:24:26] <BastyCDGS> and I confirmed it already
[15:27:30] <Vitor1001> I see
[15:47:25] <BastyCDGS> hi jai :)
[15:48:13] <BastyCDGS> do you want me removing the unused variable or can I keep the patch as is?
[15:49:39] <jai> hi
[15:49:52] <jai> i'd suggest removing the unused variable
[15:50:18] <BastyCDGS> it is used exactly one time
[15:50:26] <BastyCDGS> it starts only to become unused with the HAM patch
[15:50:40] <BastyCDGS> since I'm removing the CODEC_ID_RAWVIDEO stuff
[15:51:04] <BastyCDGS> are there any reasons, why it's better to remove it completely?
[15:53:17] <jai> i dont see the point of keeping it around only in one place
[15:54:48] <BastyCDGS> won't using st making the code a bit smaller, too?
[15:54:57] <BastyCDGS> or does the compiler recognize
[15:55:11] <BastyCDGS> that it's used multiple times and do a local storage automatically?
[16:03:25] <BastyCDGS> just checked it, using st instead of s->streams[0] makes the asm code 16 bytes shorter
[16:05:35] <kierank> it's pretty minor
[16:06:10] <BastyCDGS> yes of course, but the source code also gets smaller, that's why I ask I should do it the other way
[16:07:06] <jai> use whatever people find more readable :)
[16:07:20] <jai> i dont see this making any significant speed impact
[16:08:01] <BastyCDGS> it wasn't meant to be a speed patch more a size patch ;)
[16:08:25] <BastyCDGS> but hard to say what's really more readable here
[16:08:33] <BastyCDGS> st or s->stream[0]?
[16:08:50] <BastyCDGS> I could of course change variable name st to stream
[16:10:09] <BastyCDGS> after all st is used in iff_read_header too
[16:10:15] <BastyCDGS> so the code looks more consistent, too.
[16:10:56] <kierank> then do what's best and stop bikeshedding ;)
[16:11:38] <BastyCDGS> kierank, since the patch is already there, I'm just waiting if it's ok so or if you really want me doing another patch doing it the other way
[16:12:04] <BastyCDGS> or change st to stream...
[16:24:27] <mru> dammit, I'm committing the iff fix
[16:25:08] <BastyCDGS> mru, what's the problem?
[16:25:24] <CIA-7> ffmpeg: mru * r23063 /trunk/libavcodec/iff.c:
[16:25:24] <CIA-7> ffmpeg: IFF: decode last 8 pixels per line
[16:25:24] <CIA-7> ffmpeg: The decodeplane8() function processes one byte of input less than
[16:25:24] <CIA-7> ffmpeg: it should. Also, the for loop has an unusual style with side-effects
[16:25:24] <CIA-7> ffmpeg: in the controlling expression; replaced with a more intuitive while
[16:25:24] <CIA-7> ffmpeg: loop.
[16:25:25] <CIA-7> ffmpeg: 10l to Basty.
[16:25:26] <mru> nobody with authority to approve it replied
[16:25:50] <mru> but it's trivial enough so I committed it anyway
[16:26:07] <BastyCDGS> yes I also have doubts that someone will complain ;)
[16:26:19] <mru> never underestimate michael
[16:28:49] <BastyCDGS> just updated removal of bps patch to be applied to your dp8 fix patch
[17:06:26] <CIA-7> ffmpeg: mru * r23064 /trunk/tests/fate.mak: FATE: update idroq-video-encode command
[17:19:52] <BastyCDGS> mru, replacing the while with do while resulted in a speedup
[17:19:52] <BastyCDGS> your patch with while:
[17:19:52] <BastyCDGS> 9188 dezicycles in decodeplane8, 4095 runs, 1 skips
[17:19:52] <BastyCDGS> my test with do ... while:
[17:19:52] <BastyCDGS> 9073 dezicycles in decodeplane8, 4090 runs, 6 skips
[17:20:15] <mru> is that statistically significant?
[17:20:56] <BastyCDGS> it's 1% ;)
[17:21:01] <mru> is that statistically significant?
[17:21:21] <mru> what is the standard deviation?
[17:23:05] <wbs> BastyCDGS: make sure that even if all valid files have a width > 0, you don't loop infinitely or crash or something if you'd get a file with width==0
[17:24:40] <BastyCDGS> well since buf_size is signed now, I could just do a check >= 0
[17:28:15] <BastyCDGS> mru, lol
[17:28:20] <BastyCDGS> I just looked at disasm
[17:28:40] <BastyCDGS> the normal while patch from you changes this not to a sub but to add
[17:29:12] <BastyCDGS> i.e. gcc "optimizes" the sub 1 to add 1
[17:29:18] <mru> does it use that to index the array in the loop?
[17:29:51] <mru> that could eliminate two adds inside the loop
[17:30:02] <mru> so even with an explicit cmp you still save one insn
[17:30:14] <BastyCDGS> nope
[17:31:06] <mru> pastebin
[17:36:32] <BastyCDGS> should I pastebin both?
[17:43:32] <BastyCDGS> http://pastebin.org/213959
[17:43:52] <mru> feel free to change to a do while
[17:43:59] <mru> but then you have to check the width first
[17:44:07] <mru> actually, you should do that regardless
[17:44:28] <BastyCDGS> yes, btw, height isn't checked, too...
[17:44:40] <mru> then you should fix that as well
[17:44:50] <BastyCDGS> as 2 separate patches, right? ;)
[17:45:08] <mru> checking width and height can be the same patch
[17:45:43] <mru> hmm, your bits_per_coded_sample handling is weird too
[17:45:45] <BastyCDGS> that's clear, I meant the do while as 2nd patch
[17:45:59] <mru> or are any values allowed there?
[17:46:01] <BastyCDGS> huh? what you mean exactly?
[17:46:02] <mru> like 23
[17:46:06] <mru> or 13
[17:46:07] <BastyCDGS> yes
[17:46:12] <mru> ok
[17:46:21] <BastyCDGS> anything from 1 to 32
[17:47:20] <BastyCDGS> btw, I wanted to change that to a do while quite a long time, but totally forgotten this because of HAM stuff ;)
[17:47:32] <CIA-7> ffmpeg: mstorsjo * r23065 /trunk/tools/qt-faststart.c: qt-faststart: Avoid leaking memory if encountering a file with double ftyp atoms
[19:55:24] <peloverde> Do most people "doing video stuff on linux" have the fluendo codecs? http://www.youtube.com/watch?v=YoYL4R3Te2s#t=41m30s
[19:56:19] <elenril> lolwut?
[19:56:39] <mru> don't know if I count, but I don't
[19:57:04] <mru> in fact, I don't know _anyone_ who has the fluendo stuff
[19:57:22] <peloverde> me neither, it struck me as odd
[20:00:38] <mru> clearly a crackpot
[20:01:24] <elenril> more like amateur troll
[20:01:39] <mru> not a very good troll
[20:02:07] <mru> a good troll states undeniable truths in a very annoying way
[20:02:14] <elenril> there aren't that many good trolls
[20:02:25] * elenril wonders if that's a good thing
[20:02:37] <mru> I think it might be
[20:02:44] <mru> but damn, trolling can be fun...
[20:03:05] <spaam> mru: time for _troll_ ?
[20:03:28] <CIA-7> ffmpeg: stefano * r23066 /trunk/libavfilter/avfilter.h:
[20:03:28] <CIA-7> ffmpeg: Bump lavfi minor after the addition of the fields interlaced and
[20:03:28] <CIA-7> ffmpeg: top_field_first in AVFilterPicRef, done in r23044.
[20:07:08] <BastyCDGS> hey BBB :)
[20:07:39] * mru watches fate slowly turn green
[20:07:49] <drv> seasick? ;P
[20:08:19] <CIA-7> ffmpeg: stefano * r23067 /trunk/doc/APIchanges:
[20:08:19] <CIA-7> ffmpeg: Add entry for AVFilterPicRef interlaced and top_field_first fields
[20:08:19] <CIA-7> ffmpeg: addition.
[20:08:21] <mru> better than yellow fever
[20:08:21] <BastyCDGS> seems to be ;)
[20:08:25] <BBB> hola
[20:08:37] <BBB> sorry for breaking fate yesterday
[20:08:56] <BastyCDGS> this can happen, no thing to worry about
[20:09:04] <CIA-7> ffmpeg: rbultje * r23068 /trunk/libavcodec/iff.c:
[20:09:04] <CIA-7> ffmpeg: Remove "bps" parameter to decodeplane8/32(), it's unused.
[20:09:04] <CIA-7> ffmpeg: Patch by Sebastian Vater <cdgs basty googlemail com>.
[20:09:07] <peloverde> We need 4.5.0 on x86_*/linux
[20:11:37] <CIA-7> ffmpeg: rbultje * r23069 /trunk/libavformat/iff.c:
[20:11:37] <CIA-7> ffmpeg: Replace usage of s->streams[0]->* with st->*, which is shorter.
[20:11:37] <CIA-7> ffmpeg: Patch by Sebastian Vater <cdgs basty googlemail com>.
[20:11:56] <BBB> jai: you have a point about "just removing the variable" though, but I think in this particular case I agree that using st-> is simpler, I hope that's ok with you
[20:12:52] <BastyCDGS> not only it's simply it's also shorter asm codew
[20:12:54] <BastyCDGS> 16 bytes
[20:14:45] <BBB> right
[20:14:59] <BBB> note how this is not an inner loop so it's really irrelevant, but anyway
[20:15:03] <BBB> :)
[20:15:07] <BBB> ok, so now on to ham again
[20:15:14] <BBB> extradata really shouldn't be a multiple of 512
[20:15:17] <BastyCDGS> there will be more st's anyway when ANIM comes in ;)
[20:15:21] <BBB> it's bad habit to allocate more than needed
[20:15:41] <BBB> the compiler/system will do that for you, and will do a better job depending on pagesize etc.
[20:16:03] <BBB> (the compiler will pad, and the system will align ti pagesize)
[20:16:06] <BastyCDGS> so how many bytes I should allocate extra? 64?
[20:17:17] <wbs> that's easier for you to say than for us, try to think about what other options there are in these formats that you may need to communicate
[20:17:49] <BastyCDGS> well since I plan to support, IFF-ACBM, IFF-ANIM, IFF-DEEP, etc. it could increment pretty fast
[20:17:55] <BastyCDGS> currently it's 256 bytes
[20:17:57] <BastyCDGS> with 1 KB
[20:18:28] <Vitor1001> BastyCDGS: fate still looks failing IFF on big-endian...
[20:19:06] <BastyCDGS> vitor, the last time I checked it with be it worked
[20:19:10] <BastyCDGS> what's failing exactly?
[20:19:12] <BBB> is iff/anim still the same format though?
[20:19:25] <BastyCDGS> IFF-ANIM has a normal IFF-ILBM chunk for the first frame
[20:19:49] <BastyCDGS> the following frames are DLTA instead BODY
[20:19:52] <Vitor1001> Check FATE...
[20:19:54] <BastyCDGS> with the new compression techniques
[20:20:23] <BastyCDGS> what patches from me were you checking?
[20:20:33] <Vitor1001> For ex.: http://fate.multimedia.cx/index.php?test_result=56715528
[20:20:37] <BBB> all of them :)
[20:21:17] <BastyCDGS> you were using --enable-avfilter
[20:21:20] <BastyCDGS> IFF crashes
[20:21:29] <BastyCDGS> but the task already has assigned to somebody else
[20:21:47] <Vitor1001> works fine for me for ffmpeg (not ffplay)
[20:21:56] <BastyCDGS> also I have no glue what the problem should be, I literally have 0% experience with libavfilter
[20:29:22] <BastyCDGS> BBB, ffmpeg runs fine on m68k
[20:29:29] <BastyCDGS> ami_stuff uses it on m68k amigaos
[20:30:01] <mru> emulated iirc
[20:30:28] <mru> but there's no reason ffmpeg shouldn't work on m64k
[20:30:28] <BBB> yeah, that's not real
[20:30:29] <mru> 8
[20:30:34] <BastyCDGS> and where's the difference? if it runs on uae it runs on native amiga, too.
[20:30:41] <BBB> and it's especially no reason to align data
[20:30:46] <mru> only if the emulation is perfect
[20:30:55] * mru points at codesourcery and qemu
[20:30:57] <BastyCDGS> the 68000 is the only CPU where I know that this happens 100%
[20:31:03] <BastyCDGS> there might be other, newer ones
[20:31:13] <mru> no emulator is perfect
[20:31:19] <mru> at least no realtime usable one
[20:31:39] <BastyCDGS> WinUAE is 99,9% regarding CPU emulation, 2.x versions are even cycle-exact
[20:31:51] <mru> I don't care what you claim
[20:32:05] <wbs> BastyCDGS: yes, but still, you're using bytestream functions, they take care of unaligned reads/writes for you
[20:32:16] <mru> they probably emulate well enough that correct code runs properly
[20:32:25] <BastyCDGS> as said, there are other reasons I can't make the palette smaller than 768 bytes
[20:32:41] <BBB> how big is the palette?
[20:32:45] <BBB> is it always 768 bytes?
[20:32:51] <BBB> can it be 0, 256 or 768?
[20:32:51] <BastyCDGS> mru, then less than 5% of all amiga software (esp. games and demos) wouldn't run
[20:32:54] <BBB> can it be 33 bytes?
[20:33:02] <_av500_> 42?
[20:33:05] <BastyCDGS> it's 768 for 256 colors
[20:33:11] <mru> are you saying less than 5% of amiga software is correctly written?
[20:33:20] <mru> I knew it was bad, but _that_ bad...
[20:33:24] <CIA-7> ffmpeg: mru * r23070 /trunk/libavutil/bswap.h: bswap: 10L add missing parens around macro args
[20:33:24] <BastyCDGS> yes it can be anything from 0-768 with a multiple of 3
[20:33:37] <BBB> how free is this "number of colors"?
[20:33:40] <wbs> BastyCDGS: for the cases where data_size is smaller than one would expect, doesn't it work if one simply pads the size up to 768?
[20:33:46] <BBB> and how does the decoder know what the number of colors is?
[20:33:49] <BastyCDGS> mru, I'm talking of demos and games
[20:34:01] <mru> they still need to be correct
[20:34:02] <BastyCDGS> why do you think 99% of the old demos won't run on 68020 anymore or OS2.0+
[20:34:19] <mru> as in not doing stupid stuff that would cause cpu exceptions
[20:34:33] <BastyCDGS> BBB, by extradata_size
[20:34:41] <BastyCDGS> it divides it by 3 to get number of palette entries in CMAP
[20:35:23] <BastyCDGS> mru, some demos/games even rely that 68000 throws exception on unaligned access
[20:35:37] <BastyCDGS> they use this to jump into supervisor mode
[20:35:40] <BastyCDGS> stupid I know
[20:35:52] <mru> that's what caught codesourcery
[20:36:00] <mru> qemu didn't emulate unaligned traps correctly
[20:36:30] <mru> if you're only trying to execute well-behaved code as fast as possible, that's perfectly reasonable too
[20:36:30] <BastyCDGS> wbs, the problem here is that the old decoder must conform with new demuxer and vice versa
[20:36:52] <wbs> BastyCDGS: yes, but what harm would it do if you feed a larger cmap to the older decoder?
[20:37:01] <wbs> you will just get a slightly larger palette with extra black items at the end
[20:37:16] <wbs> which shouldn't affect the decoding of an image that just uses the earlier palette entries?
[20:37:34] <BastyCDGS> this won't work since the older decoder reads these entries from the file
[20:37:45] <BastyCDGS> if the CMAP chunk is the very last one it will break (trying reading after EOF)
[20:37:52] <wbs> no it doesn't read it from file
[20:37:56] <wbs> it reads it from extradata
[20:38:11] <BastyCDGS> oh yes sorry was mixing it up with the RAWVIDEO stuff
[20:38:20] <wbs> if you have a new demuxer which reads a too short cmap, but nevertheless pads the palette to 768 bytes
[20:39:16] <wbs> so can we agree that in the decoder, you don't need to know the actual amount of palette entries set, when the demuxer adds a full palette in extradata?
[20:40:37] <BastyCDGS> I know that I tried a lot of stuff before doing it the way as I did now
[20:40:51] <wbs> that doesn't make it the only way things could work
[20:40:55] <BastyCDGS> everything else I tried broke here or there
[20:41:02] <wbs> cause the current proposal still has a lot of flaws
[20:41:32] <BastyCDGS> the thing is that the old decoder checks the size of extradata
[20:41:40] <BastyCDGS> and throws a palette underflow
[20:41:52] <BastyCDGS> but doesn't handle palette overflow
[20:41:58] <wbs> no, but it doesn't have to handle it either!
[20:42:09] <wbs> you just get a few extra palette entries that won't ever be used
[20:42:13] <BastyCDGS> it has, I get segfault otherwise
[20:43:03] <BastyCDGS> the filling remaining with black was one of the very first things I tried
[20:43:46] <wbs> this particular sample, sabre.iff, does it use HAM, or should it be decodable with the current decoder if it didn't have too short cmap data?
[20:43:50] <BastyCDGS> believe me, I'm not happy with the current hack either
[20:44:07] <BBB> I don't get it
[20:44:10] <BBB> can you have 4 colors?
[20:44:11] <BBB> or 7?
[20:44:12] <BastyCDGS> it doesn't use HAM...it just has a shorter color map
[20:44:15] <BBB> or only 2^n?
[20:44:36] <BastyCDGS> IFF specs say that if the CMAP chunk has less entries than 1 << bps it has to be considered black
[20:44:47] <BastyCDGS> if no CMAP at all fill with greyscale indices
[20:45:04] <BastyCDGS> but be careful, when using HAM the image can still be color
[20:46:32] <wbs> BastyCDGS: ok, so if sabre.iff can be decoded with the current decoder, would this work? http://pastebin.org/214566
[20:47:01] <wbs> or something similar, padding the extradata with extra black entries in the demuxer
[20:49:27] <BastyCDGS> yes that should work
[20:49:42] <BastyCDGS> the thing is only that the avformat can be changed independently of avcodec
[20:49:51] <BastyCDGS> and still have to be compatible
[20:49:59] <wbs> ... yes, i frickin know that
[20:50:05] <BastyCDGS> also your patch does evaluate the * 3 and the && each loop iteration
[20:50:29] <wbs> stop micro-optimizing everything, we're discussing high-level stuff here
[20:50:43] <wbs> the question was, does this work or not
[20:51:10] <BastyCDGS> as long as new decoder and new demuxer are the same, yes...
[20:52:04] <wbs> so, since this works, it should work if we'd just adjust the demuxer to set a larger extradata, too, to include the full nominal size of the cmap data, even if the actual data read from file doesn't populate all of it, right?
[20:53:33] <BastyCDGS> this will segfault if new demuxer doing this will meet old decoder...
[20:53:41] <BastyCDGS> as said, old decoder doesn't handle overflow :(
[20:53:52] <BastyCDGS> the thing is you can write only up to 1 << bps
[20:54:03] <BastyCDGS> palette entries anything after it will segfault
[20:54:04] <wbs> how on earth would it segfault? could you please describe that to me
[20:54:06] <wbs> or stop bullshitting
[20:54:25] <BastyCDGS> because ffmpeg just allocates 1 << avctx->bits_per_coded_sample entries?
[20:54:28] <wbs> yes
[20:54:45] <BastyCDGS> so if bps == 6 there are 64 color entries allocated
[20:54:53] <wbs> yes, and extradata happens to contain 256 color entries
[20:54:55] <BastyCDGS> the old decoder with your new demuxer would try to write 256...
[20:55:06] <wbs> no it wouldn't
[20:55:15] <wbs> it would try to read 64 color entries out of 256 and just ignore the rest
[20:56:20] <BastyCDGS> just opened an old version, you seem be right
[20:56:38] <wbs> yes, as a matter of fact, I happen to be very right on that account
[20:57:34] <wbs> so can we all agree that the demuxer is completely allowed to return a full 256 color palette, even if the file actually contained much fewer colors?
[20:57:46] <wbs> and doing it that way would solve sabre.iff without touching the current decoder
[20:58:13] <BastyCDGS> ahh now I understand, you just want to fix sabre...
[20:58:39] <wbs> no, I want you to get a sane way of passing the new extradata between demuxer and decoder
[20:58:50] <wbs> but to even be able to discuss that with you, you need to realize these things first
[20:58:56] <wbs> so you won't get hang up on them later
[20:59:40] <wbs> and if we can fix sabre.iff with a small <5 lines patch now, it is MUCH easier to relate on how the new demuxer/decoder should behave, if we don't change 15 things of behaviour at once, but just fix ONE ISSUE AT A TIME
[21:01:24] <BastyCDGS> but I'ld like to replace:
[21:01:24] <BastyCDGS> i < count && i*3 < avctx->extradata_size
[21:01:24] <BastyCDGS> with:
[21:01:24] <BastyCDGS> count = FFMIN(avctx->extradata_size / 3, count);
[21:01:24] <BastyCDGS> and keep the for loop as it
[21:01:45] <BastyCDGS> as is
[21:02:10] <wbs> yes, that's micro-optimizing stuff, I don't care about that now
[21:03:34] <BastyCDGS> and if no CMAP is there, we fill in the grayscale map from demuxer ;)
[21:04:47] <BastyCDGS> so we can fix that, too, without touching the decoder, really neat!
[21:09:53] <wbs> so, then the new extradata parameter passing stuff, would boil down to this:
[21:09:56] <wbs> http://pastebin.org/214654
[21:10:45] <wbs> so, if the cmap extradata happens to be shorter than expected, we just extend it to the full nominal size, and then append the new parameters at the end
[21:11:02] <BBB> I think that's a logical approach
[21:11:23] <wbs> and the new decoder can likewise check if the extradata happens to be longer than what the nominal size would be, and then parse out the parameters from there
[21:11:38] <BBB> I think that's the ideal approach
[21:11:58] <BBB> and as long as we keep the order of parameters fixed (and documented), we don't need padding of, say, 20 bytes
[21:12:05] <BBB> we can just allocate nominal size + 5 now
[21:12:09] <BBB> and fill in the 5 etxra bytes
[21:12:18] <wbs> exactly, then we don't need to preallocate anything extra at all
[21:12:32] <BBB> alloc, followed by realloc, is ugly btw
[21:12:36] <BBB> but let's not get into that for now :)
[21:13:03] <wbs> yeah, you can fix that in another way later if you want to, but this kept the pseudocode patch small :-)
[21:13:25] <BastyCDGS> agree...sorry I didn't understand from the very first time
[21:13:44] <BastyCDGS> should I separate the decoder FFMIN patch for palette?
[21:13:57] <BastyCDGS> and the always maximum alloc of 768?
[21:14:07] <wbs> one theoretical problem, though; an old demuxer reading an oversized cmap, would be interpreted as new-format extra parameters
[21:14:10] <BBB> not 768
[21:14:24] <BBB> 3<<bps, right?
[21:14:30] <wbs> no, that was just a theoretical mind-exercise leading up to the example solution above
[21:14:31] <BBB> or 3<<(bps-1)
[21:14:42] <wbs> but yeah, if you want to, you can do it in such steps
[21:15:20] <wbs> i quite frankly don't care, as long as you end up with a sensible solution
[21:15:30] <BastyCDGS> wbs, you mean an old decoder, instead old demuxer, right?
[21:15:39] <BastyCDGS> Martin cares about this...
[21:15:46] <BBB> martin = wbs :)
[21:15:59] <BastyCDGS> oh...didn't know that
[21:16:03] <BBB> and he means new decoder + old demuxer reading an oversized cmap chunk
[21:16:07] <wbs> yeah
[21:16:20] <BBB> and yeah, that's a bug and we don't care since people should simply update their decoder
[21:16:23] <BBB> as long as it doesn't crash
[21:16:26] <wbs> but that falls into the category "malformed input" imo
[21:16:32] <BBB> right
[21:17:16] <wbs> as long as proper data is handled properly with mismatched lavf/lavc versions (but doesn't crash with malformed data either), I don't think it's that big an issue if handling of fringe cases is a bit spotty
[21:18:13] <wbs> we wouldn't want to build some really horrible contraption just to take care of that
[21:19:25] <BBB> not for fringe formats at least
[21:19:38] <_av500_> ffringe
[21:19:44] <BBB> :)
[21:19:46] <wbs> haha
[21:20:05] <BBB> ok, keep the patches coming BastyCDGS, it's looking better than before and we'll get it in in a bit
[21:20:15] <BBB> I applied 2, will apply more (probably) tomorrow
[21:21:29] <BastyCDGS> http://pastebin.org/214703
[21:21:30] * BBB goes work a little more
[21:21:32] <BastyCDGS> this patch ok?
[21:22:20] <wbs> i'd say so
[21:22:30] <BastyCDGS> why?
[21:22:57] <wbs> uh, please reread what I said
[21:22:58] <BBB> it results in uninitialized pal[] entries if count<2^n
[21:23:09] <BBB> is there code that initializes them to b/w later?
[21:23:21] <BastyCDGS> isn't the palette av_mallocz'd?
[21:23:38] <BBB> right, but they shouldn't be 0x00000000
[21:24:11] <BastyCDGS> they should be 0
[21:24:13] <wbs> i meant "yes, i think that is ok, but it'd be good to wait for a comment from someone else, too"
[21:24:24] <BBB> they should be (0xff*i/(1<<bps))*0x01010101
[21:24:24] <BastyCDGS> only if no CMAP at all
[21:24:46] <BBB> hm, that's counterintuitive
[21:24:53] <BastyCDGS> it's the IFF standard sorry
[21:24:57] <wbs> BBB: that's the case we discussed earlier, if the file happened to contain a too small palette (since the image doesn't use the full palette), leaving the rest of them to 0 is ok
[21:25:00] <BBB> ok, then it's fine
[21:25:12] <BBB> maybe add a comment of 1 line that says that
[21:25:17] <BBB> so people reading the code see that too
[21:25:22] <BastyCDGS> it's better to do greyscaling in a separate patch, right?
[21:25:27] <BBB> yes
[21:25:42] <wbs> one issue per patch, yes
[21:25:42] <BBB> add a 1-line comment and then the patch is ok
[21:25:54] <BBB> I'll be back in a bit
[21:27:04] <BastyCDGS> http://pastebin.org/214716
[21:27:08] <BastyCDGS> so updated
[21:29:09] <BastyCDGS> submitted to ml
[21:32:43] <wbs> i'm off to bed now, but if you get the parameter passing rewritten in the way outlined above, I think most people will be very much more willing to accept it
[21:33:05] <BastyCDGS> okey then good night and nice dreams ;)
[21:33:07] <BastyCDGS> and thank you
[21:33:16] <BastyCDGS> I'll go to bed soon, too...
[21:35:20] <wbs> you're welcome
[21:36:16] <BastyCDGS> thank you again :)
[21:56:54] <j-b> ramiro: ping. Do you know somehting about an ebp issue when Xcompiling to Win32?
[21:58:45] <BastyCDGS> j-b are you compiling with enable-pic?
[21:59:18] <j-b> BastyCDGS: no.
[21:59:24] <BastyCDGS> hmm strange...
[21:59:38] <j-b> ffmpeg configure tells me that ebp is not usable
[21:59:53] <mru> configure does not lie
[22:00:32] <j-b> mru: :) but mingw is buggy => hence question to Mr. ramiro
[22:08:27] <ramiro> j-b: what kind of ebp issue?
[22:09:04] <j-b> ramiro: when using mingw32-gcc 4.4.2, ffmpeg configure tells me ebp is unusable
[22:09:15] <j-b> http://vlc.pastebin.com/tJhdZ6pv
[22:09:35] <j-b> the relevant part of config.err ^^
[22:09:41] <j-b> and  http://vlc.pastebin.com/5cYS6iKF is the result.
[22:10:00] <j-b> ramiro: since I am a bit stupid, I prefer to ask before I prepare my next package :)
[22:10:50] <ramiro> I get that too, and I just checked that it was available on 4.2
[22:11:19] <ramiro> it's something gcc related, I don't really know what makes gcc decide bp can't be used.
[22:12:18] <mru> phase of the moon most likely
[22:13:43] <j-b> ramiro: ok, many thanks
[22:13:59] <ramiro> I'd really like to know why though...
[22:14:00] <j-b> mru: impossible, that would mean that they would be good days
[22:14:33] <j-b> ramiro: well, i am preparing libavcodec package for VLC 1.1.0 release, so I would prefer to avoid making too many mistakes...
[22:15:51] <ramiro> j-b: I understand
[22:15:56] <mru> j-b: it does the right thing only when the back side of the moon is visible
[22:15:58] <ramiro> btw what --cpu do you use for x86?
[22:16:08] <ramiro> I mean 32-bit x86
[22:16:24] <BastyCDGS> mru, lol
[22:16:41] <j-b> ramiro: --cpu=i686 -
[22:16:47] <BastyCDGS> so I have to travel in space and run gcc there? :D
[22:16:49] <mru> j-b: try playing some pink floyd while compiling
[22:16:51] <ramiro> mru: I'm sure you must know the real reasons why gcc does that, could you please at least point us to the right direction?
[22:16:57] <mru> dark side of the moon might be good enough
[22:17:01] <BastyCDGS> mru, I love pink floyd :)
[22:17:09] <j-b> ramiro: --enable-cross-compile --target-os=mingw32 --arch=x86 --enable-memalign-hack --cpu=i686  is the relevant part
[22:17:23] <BastyCDGS> shine on you crazy gcc might also be a good song from pulse ;)
[22:17:36] <j-b> mru: I _was_ actually listening to this album... Where are you ? Behind my shoulder?
[22:18:03] <BastyCDGS> he's telepathic ;)
[22:18:23] <j-b> ramiro: and using SVN from a few days ago
[22:19:12] <ramiro> j-b: a few days ago?! are you kidding us?! do you really expect help from us by using such an ancient version?
[22:19:15] <ramiro> =)
[22:20:19] <BastyCDGS> no he just travelled faster than light to moon to check if it works when he sees back side...so he travelled to past time at the same time :D
[22:20:20] <j-b> ramiro: =) I prefer to use not the last 72hours ones, usually, so I can see regression complaints on the mailing list
[22:56:55] <CIA-7> ffmpeg: stefano * r23071 /trunk/libavformat/nutdec.c:
[22:56:55] <CIA-7> ffmpeg: Make the nut demuxer issue a more meaningful error message if it
[22:56:55] <CIA-7> ffmpeg: cannot recognize the provided codec tag.
[22:56:55] <CIA-7> ffmpeg: stefano * r23072 /trunk/ (libavformat/riff.c libavcodec/raw.c):
[22:56:55] <CIA-7> ffmpeg: Add support to the Y411 codec tag, corresponding to the rawvideo pixel
[22:56:56] <CIA-7> ffmpeg: format uyyvyy411.
[22:56:57] <CIA-7> ffmpeg: The codec tag is referenced in fourcc.org.
[23:05:38] <CIA-7> ffmpeg: stefano * r23073 /trunk/libavcodec/raw.c:
[23:05:38] <CIA-7> ffmpeg: Make the codec tags for the yuvjXXX pixel formats the same as the
[23:05:38] <CIA-7> ffmpeg: corresponding ones for the yuvXXX pixel formats.
[23:05:38] <CIA-7> ffmpeg: stefano * r23074 /trunk/libavcodec/raw.c: (log message trimmed)
[23:05:38] <CIA-7> ffmpeg: Add missing nut-specific codec tags for rawvideo pixel formats.
[23:05:39] <CIA-7> ffmpeg: Add codec tags for the formats:
[23:05:40] <CIA-7> ffmpeg: [15]BGR Packed RGB 5:5:5, 16bpp, (msb)1A 5R 5G 5B(lsb), big-endian [NOT in AVI]
[23:05:40] <CIA-7> ffmpeg: [15]RGB Packed BGR 5:5:5, 16bpp, (msb)1A 5B 5G 5R(lsb), big-endian [NOT in AVI]
[23:05:41] <CIA-7> ffmpeg: [16]BGR Packed RGB 5:6:5, 16bpp, (msb) 5R 6G 5B(lsb), big-endian [NOT in AVI]
[23:05:41] <CIA-7> ffmpeg: [16]RGB Packed BGR 5:6:5, 16bpp, (msb) 5B 6G 5R(lsb), big-endian [NOT in AVI]
[23:05:50] <CIA-7> ffmpeg: stefano * r23075 /trunk/libavcodec/raw.c: (log message trimmed)
[23:05:50] <CIA-7> ffmpeg: Reorder nut specific codec tags and add a comment for marking them as
[23:05:50] <CIA-7> ffmpeg: such.
[23:05:50] <CIA-7> ffmpeg: Also put the [3][0][0][0] codec tag, mapped to rgb565le, in a special
[23:05:51] <CIA-7> ffmpeg: section. It needs to be specified *after* the nut RGB[16] codec tag,
[23:05:51] <CIA-7> ffmpeg: otherwise it will be used by default when encoding normal non-flipped
[23:06:18] <CIA-7> ffmpeg: rgb565le, and will be decoded like a flipped format (see
[23:22:21] <CIA-7> ffmpeg: cehoyos * r23076 /trunk/libavformat/riff.c:
[23:22:21] <CIA-7> ffmpeg: Add FourCC MJPG for CODEC_ID_JPEGLS.
[23:22:21] <CIA-7> ffmpeg: Patch by Francesco Lavra, francescolavra interfree it