Bugzilla – Bug 59
No error reported to user and crash when FFmpeg attempts invalid custom export of WAV with mp2 codec
Last modified: 2019-09-01 14:15:55 UTC
* GA 02Jan10: But how many format/codec combos work?? On Windows, I tried WAV format with wmav1, pcm_alaw, adpcm_ima_wav, mp2 and libmp3lame, and all are 0 bytes (including metadata or not). Ogg with FLAC works. WAV/mp2 is 0 bytes on Ubuntu (didn't try others). o JC 06Jan10: We should add a test that the resulting file is not zero bytes. Then we can at least report "File not exported. FFmpeg did not succeed with that choice of settings". o LRN: Back when i wrote the exporter, i planned to report all ffmpeg errors caught by Audacity after the export procedure completes (successfully or not), so that user doesn't have to look at debug log to see the problem. That would work well when (wow, four w-words in a row!) coupled with zero-file-size check, i believe.
(In reply to James Crook from comment #0) there are far too many combinations to be able to find the time (for me certainly) to test adequately
*** STEPS UPDATED *** Steps updated, so that we have a testable scenario that we have some hope of closing. We could close this bug either by reporting errors after a failed export, or by indicating in advance to the user that certain combinations won't work, e.g. by not listing them. The easier fix is reporting on failed exports, and is the most likely one we'll take.
Testing with W10 audacity-2.3.0-alpha-120-b881b06b5347fabe0903bea447a313e077be4512 2a) works fine and produces a playable file 2b) crashes Audacity - produces a short unplayable file
Created attachment 823 [details] Custom FFmpeg - default Codec options for WAV Format Testing on W10 with 2.3.2 RC001 In Custom FFmpeg export selecting WAV as the Format the list of suitable Codecs does not contain mp2 - so by default the subject of this bug is an unachievable combination. You can however force the issue by clicking on "Select All Codecs" - this then shows mp2 as an available (albeit unsuitable) codec and selecting mp2 there wit WAV as the Format will indeed cause a crash - and with no error message. Since you have to somewhat force the issue we have to ask ourselves is this really a bug ? It is bad that it causes an error-message-less crash though - we should not be seeing that. ------------------------------------------------ I recall that we had a discussion a while back about the combinations of Formats and Codecs for Custom FFmpeg export - and decided back then that there were far too many combinations to test thoroughly. And many of those may also cause error-message-less crashes or corrupted output data
Janes wrote in comment #0 >JC 06Jan10: We should add a test that the resulting file is not zero bytes. >Then we can at least report "File not exported. FFmpeg did not succeed with >that choice of settings". and James later wrote in Comment #2 >We could close this bug either by reporting errors after a failed export, >or by indicating in advance to the user that certain combinations won't work, >e.g. by not listing them. The easier fix is reporting on failed exports, >and is the most likely one we'll take. So can we please implement the 0-byte failed export test and notify the user with an error message.
raising this to P2 as this could easily result in data loss if a) the user makes such a failed export (but believes it to be exported ok) b) exits Audacity without Saving the (unrepeatable) project c) later tries to play the 0-byte file
(In reply to Peter Sampson from comment #6) > ... this could easily result in data loss if > a) the user makes such a failed export (but believes it to be exported ok) > b) exits Audacity without Saving the (unrepeatable) project > c) later tries to play the 0-byte file This is in fact not an issue as on both W10 and mac with latest 2.3.3 alphas Audacity crashes silently with no error message and no crash log - so the user knows it has gone awry. The project can be recovered successfully on next launch of Audacity. But the Bug remains at P2 due to the crash - It would be P1 but for the fact that >Custom FFmpeg Export >Header = WAV >Codec = MP2 is somewhat of a fringe case.
Note that although this bug title is specifically WAV/mp2 - the release note probably correctly observes: >Custom FFmpeg Export: many combinations of formats and codecs are incompatible, >as are some combinations of general options and codecs. Some files may be >exported as zero kb files. Some incompatible format/codec combinations may >crash Audacity And note that there are far too many combinations of header and codec for us to contemplate exhaustive testing.
line 655 in ExportFFmpeg.cpp is calling the ffmpeg dll and causing an exit (aka crash) ret = avcodec_encode_audio2(avctx, pkt, frame.get(), &got_output); A try-catch will not catch this. So either we have to compile ffmpeg dll ourselves and ensure it does not exit, or catch the invalid combination ourselves sooner.
DEVEL - FIX MADE https://github.com/audacity/audacity/commit/089a7ec16609a9c90ebf1e6354fc31cb1adf0fac There are surely 'residuals' for this bug, but probably they are lower priority than P2 and merit a new bug with new steps.
Testing on W10 with audacity-2.3.3-alpha-337-089a7ec16609a9c90ebf1e6354fc31cb1adf0fac When using Custom FFmpeg export and select WAV as the Format - there is no longer by default an entry for MP2 - so in normal use this error is avoided. However, having select WAV as the Format and then clicking "Show All Codecs" mp2 then becomes avilable -selecting that codec and pressing "OK" to export the file results are: 1) No error message 2) brief blue=spinning-circle-of-death 3) silent crash of Audacity 4) null 1kb file exported
*** STEPS UPDATED *** Added step of 'Show all Codecs' which is now essential. In effect the user is bypassing guidance to not use MP2 with wav. Hence downgrading from P2 to P3, as the user has opted to ignore the guidance.
Added a workaround - advising don't use "Show All Codecs"
DEVEL - FIX MADE https://github.com/audacity/audacity/commit/1a49d0a81231981b7a2f3e3d11212cbdcd03652d
(In reply to James Crook from comment #14) Testing on W10 with audacity-2.3.3-alpha-341-72f36f8a82e9ed4a184b9029cb53780e62a9bea8 MP2 is disabled and no longer appears in list of all codecs - thus this prevuious condition can no longer aooly.
