Canardoux / flutter_sound

Flutter plugin for sound. Audio recorder and player.
Mozilla Public License 2.0
869 stars 568 forks source link

[BUG]: stopRecorder returning empty Url of recorded audio on iOS, working fine on android. #676

Closed themaaz32 closed 3 years ago

themaaz32 commented 3 years ago

Flutter Sound Version :

Full

flutter pub deps | grep flutter_sound |-- flutter_sound_lite 8.1.1 | |-- flutter_sound_platform_interface 8.1.1 | |-- flutter_sound_web 8.1.1 | | |-- flutter_sound_platform_interface...


Severity


Platforms you faced the error


Describe the bug stopRecorder returning empty Url of recorded audio on iOS, working fine on android.

To Reproduce

Future record() async {
    final isGranted = await Permission.microphone.request();
    if(isGranted.isGranted){
      await _flutterSoundRecorder.openAudioSession();
      await _flutterSoundRecorder.startRecorder(toFile: "${DateTime.now().toString()}.mp3");
    }else{
      print("Permission not granted");
    }
}
  Future<String> stopRecording()async{
    final url = await _flutterSoundRecorder.stopRecorder();
    await _flutterSoundRecorder.closeAudioSession();

    print(url);
    return url //url = "" in ios;
  }

calling stopRecording returning empty string only in ios.


Logs!!!!

flutter: FS:---> openAudioSession
flutter: ---> openAudioSession
flutter: Resetting flutter_sound Recorder Plugin
@resetPlugin
iOS: ---> resetPlugin
iOS: <--- resetPlugin
@openRecorder
IOS:--> setAudioFocus
IOS:<-- setAudioFocus
flutter: <--- openAudioSession
flutter: ---> openRecorderCompleted: true
flutter: <--- openRecorderCompleted: true
flutter: FS:<--- openAudioSession
flutter: FS:---> startRecorder
flutter: FS:---> _startRecorder.
flutter: Calling instance.startRecorder
flutter: ---> startRecorderCompleted: true
flutter: <--- startRecorderCompleted: true
flutter: FS:<--- _startRecorder.
flutter: FS:<--- startRecorder
flutter: recording start
flutter: FS:---> stopRecorder
flutter: FS:---> stopRecorder
flutter: FS:---> stop
iOS ---> stopRecorder
iOS <--- stopRecorder
flutter: ---> stopRecorderCompleted: true
flutter: <---- stopRecorderCompleted: true
flutter: FS:<--- stop
flutter: FS:<--- stopRecorder :
flutter: FS:<--- stopRecorder
flutter: FS:---> closeAudioSession
flutter: FS:---> closeAudioSession
flutter: FS:---> stop
iOS ---> stopRecorder
iOS <--- stopRecorder
flutter: ---> stopRecorderCompleted: true
flutter: <---- stopRecorderCompleted: true
flutter: FS:<--- stop
iOS ---> closeRecorder
iOS: ---> releaseSession
iOS: <--- releaseSession
iOS <--- closeRecorder
flutter: ---> closeRecorderCompleted
flutter: <--- closeRecorderCompleted
flutter: FS:<--- closeAudioSession
flutter: FS:<--- closeAudioSession
flutter: [URL expected but empty]

MaekawaTakahiro commented 3 years ago

It has been updated from 8.1.4 to 8.1.9 and now returns the URL path, but I don't think the file is being saved correctly. What is the status of this bug?

Larpoux commented 3 years ago
  await _flutterSoundRecorder.startRecorder(toFile: "${DateTime.now().toString()}.mp3");

mp3 is not a supported codec for recording. Neither on Android nor iOS

mhstoller commented 3 years ago

@Larpoux if the user provides a toFile parameter that ends with .mp3 does the library automatically try to use mp3 codec instead of the default (AAC)?

Is recording to mp3 supported on any of the platforms? Maybe I can look into raising an exception in the dart code when trying to record to mp3 (might be a good first contribution). I've seen a few of these very similar tickets opened

Larpoux commented 3 years ago

Yes @mhstoller , You are right : probably better if Flutter Sound throws an understable exception when the user tries to record something which has a mp3 extension.

Mp3 cannot be recorded, neither on iOS nor Android. MP3 is (was?) not a free codec, and we must have a Thomson license to record mp3. One of my many project is to allow recoding MP3, using the "lame" library. (Actually Flutter Sound is linked with Mobile FFmpeg, and Mobile FFmpeg includes the "lame" library).

I told @themaaz32 that he/she is wrong trying to record Mp3, but I think that it is not really the problem. Actually Flutter Sound does not use the file extension to select the Codec. It uses a codec: parameter and this parameter has a default value AAC. Another solution is to have the Codec parameter mandatory and not optional. I would prefer this solution but this would be a breaking change

But I really agree with you : if the extension is incorrect, probably the developer is trying to do something wrong and we must display at least a Warning. Actually I am busy on implemented a smart "Logger" : the Developer will be able to choose if he/she wants to have the full debugging logs, or just Errors and Warnings. Please keep this issue open : someone should do something for those kinds of problems.

BTW : @mhstoller : you are great 👍 . You spent time yesterday on another Problem Report, and you tried to execute the @AbdallahLabib's Code. I really appreciate that you spend time on Flutter Sound maintenance. There is not enough developers involved in the Flutter Sound maintenance and development.

mhstoller commented 3 years ago

Let me look through the code and see if I can find an acceptable solution to close this. I will come back to this later.

Thanks for the kind words

Larpoux commented 3 years ago

My preference is that the Codec parameter must be mandatory and not optional. If the developer really wants the "defaultCodec" value, he/she will have to say it explicitly.

I plan to work soon (?) on cleaning the Flutter Sound API. This kind of Problem Report will probably be addressed int this new clean API.

Larpoux commented 3 years ago

The new API will not be a complete breaking change. I will keep the actual FlutterSoundPlayer and FlutterSoundRecorder API as deprecated modules.

New developers will use the TauPlayer and TauRecorder modules. I would like to do this new API before working on ReactNative API. But I have currently problems with my plannings. I do not work enough on Flutter Sound and on other projects that I develop in parallel . I am too slow in my developments 👎

mhstoller commented 3 years ago

@Larpoux

That sounds good, however the file extension of mp3 will still not work on iOS even if the codec is AAC. I'm not sure if thats a flutter_sound issue or iOS issue, but the output file is null when the file extension is mp3.

My suggestion since there is not current plans to add mp3 recorder support soon is to add another check in flutter_sound_recorder.dart

There is a section of code that checks for valid stream codec already, I can add another check after it:

    if ((toFile == null && toStream == null) ||
        (toFile != null && toStream != null)) {
      throw Exception(
          'One, and only one parameter "toFile"/"toStream" must be provided');
    }

    if (toStream != null && codec != Codec.pcm16) {
      throw Exception('toStream can only be used with codec == Codec.pcm16');
    }

Or similar to isEncoderSupported there can be a function that checks that the file extension is correct for the codec before checking if it's supported. So in this case, when the user tries to record with AAC codec, but the extension is .mp3 it can throw an exception or a warning.

Thoughts?

Larpoux commented 3 years ago

My suggestion since there is not current plans to add mp3 recorder support soon is to add another check in flutter_sound_recorder.dart

YES!

Definitely better. If you want to be perfect, it would be to check if the extension is consistency with the Codec selected by the developer. For example : toFile: 'foo.opus' and codec: AAC is probably not what the developer wants.

Be careful : some several extensions are allowed for a specific Codec. For example foo.opus and foo.ogg are both correct for Codec OPUS.

I suggest that it is better to display a Warning than throwing an exception. But if you are doing a fix for this Problem Report then YOU are now the boss for this subject 👁️

Larpoux commented 3 years ago

NO. I am wrong. It is not a warning but an exception. because your investigations showed that iOS does not record *.mp3

mhstoller commented 3 years ago

I have a lot of work to compile the codec file extensions I don't see a good resource that has them compiled already. I'm going to start this and I think I will use an assertion instead of throwing an exception. This way, if I miss a valid file extension it won't affect production apps, only apps being debugged and hopefully the dev will report the issue to us and we can update the list.

It will still help devs new to Tau, I think this is safer.

mhstoller commented 3 years ago

@Larpoux

How would you prefer the opus codecs to be handled? Should the file extension for Codec.opusOGG only have '.ogg' as valid, or should I build it so all opus codecs accept all valid extensions? And same for others like pcm16CAF, pcm16WAV, etc.

Should I do this:

case Codec.opusCAF:
case Codec.opusOGG:
case Codec.opusWebM:
    _validExtensions = [
            '.opus',
            '.ogg',
            '.mka',
            '.mkv',
            '.caf',
            '.webm',
            '.ts'
          ];
    break;

or this:

case Codec.opusCAF:
    _validExtensions = '.caf';
    break;
case Codec.opusOGG:
    _validExtensions = '.ogg';
    break;
case Codec.opusWebM:
    _validExtensions = '.webm';
    break;
Larpoux commented 3 years ago

case Codec.opusCAF: _validExtensions = '.caf'; break; case Codec.opusOGG: _validExtensions = '.ogg'; break; case Codec.opusWebM: _validExtensions = '.webm'; break;

This seems good to me. But now you are the boss 😆 .

Larpoux commented 3 years ago

@mhstoller ,

Your Pull Request is implemented in v.8.2.6. I will release this version in a few days. Thank you again for your contribution. I really appreciate that you help me doing Flutter Sound Customer(?) Support.