Le nonidi 9 germinal, an CCXXIII, Stephan Holljes a écrit : > I hope this addresses the issues mentioned. > I added a new label in case of failure in http_open() in favor of > duplicated code (i.e. calling av_dict_free() multiple times). I copied > the style from the other functions. > > Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>
The patch looks good to me (acronym to learn: LGTM) as is or with the code moved into a separate function point. Except one more point that was not addressed earlier: if you want to add comments to a Git patch email, you need to put them between the "---" and the first "diff --git" lines. That is where the small stats "2 files changed, 28 insertions(+), 1 deletion(-)" is already inserted, it is considered as a simple comment too. If you put them before the "---" line, it becomes part of the commit message when the patch is applied from the mail. For the final version, maybe better attach the patch or push it to a public clone. At this point, I suspect you can consider this will be applied soon (better give it maybe two days before asking for merging) and, if you want to strengthen your application, make the patch even better, for example getting it to work for input as well as output. For that, I suspect it will be convenient for you to isolate the code in its own function, that makes handling the variables and the failure code path easier, so you probably better follow wm4's advice. Regards, -- Nicolas George
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel