On Wed, Mar 04, 2015 at 02:19:36PM -0400, Peter Cordes wrote: > On Wed, Mar 4, 2015 at 9:42 AM, Michael Niedermayer <michae...@gmx.at> > wrote: > > > On Wed, Mar 04, 2015 at 02:24:58PM +0100, Michael Niedermayer wrote: > > > On Wed, Mar 04, 2015 at 02:11:42PM +0100, Michael Niedermayer wrote: > > > > On Tue, Mar 03, 2015 at 10:51:01PM -0400, Peter Cordes wrote: > > > > [...] > > > > > Anyway, the av_dict change is the one that requires the most review, > > so > > > > > I'll make this email focus on that set of changes. > > > > > https://github.com/FFmpeg/FFmpeg/pull/118 > > > > > > > > pull req #3, patch #1 review > > > > > > > > > - char *ret = out, *end = out; > > > > > + char *ret = out, *end_quote = out; > > > > > > > > why ? > > > > Because it took me a couple minutes to see that was all "end" was doing. I > wasn't sure what it was the end of. (At first, I didn't even realize that > this function was handling quoting as well as tokenizing.) > > > > > > + ret = av_realloc(ret, out - ret + 2); > > > > > + // if realloc fails to shrink, we're hosed anyway; just leak > > the old buffer > > > > > return ret; > > > > Yeah, I should have collapsed that bad idea / fix before pushing. I > wanted to leave the code shorter, but then I realized I was starting to > write a whole paragraph in a commit message to justify a one-line shortcut, > so it was probably a bad idea. :P > > > > if realloc fails to shrink, the original unshrunk array should be > > > > returned to avoid the leak and failure > > > > > > ahh, i see you fix this in a later commit, this should be stashed > > > with the patch that would add the bug if it was pushed > > > > and now i see you mentioned that in your mail, i guess replying to > > the first part and then reviewing some of the code before reading > > the rest of the mail was not really a great idea > > > I have ADHD, I have a hard time keeping my emails under 10 paragraphs of > different ideas. :P > > My main desktop just developed some sort of instability, maybe hardware. > It might be a few days before I update these patches, if I don't get to it > on my laptop. > > I guess ffmpeg prefers splitting changes into MANY separate commits for > more accurate bisection. I knew that, but didn't realize what level of > splitting up was aimed for. Will do for all my other patches, now that I > have a better idea of what might actually get accepted. (esp. the > libx264.c commit has at least 3 commits worth of changes.) > > Several of the patches on my other branches are already single-item changes > that are ready to go in, though. (Other than the mpdecimate one where I > thought there was a bug passing passing an invalid pointer as a log > context. I'm still learning my way around ffmpeg. Thanks for the patch > review on that.) >
> Especially the vf_showinfo change is simple and useful. applied this and 3 other simple patches thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 1 "Used only once" - "Some unspecified defect prevented a second use" "In good condition" - "Can be repaird by experienced expert" "As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
