Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added ability to create audiosegment from a segment of a file #345

Merged
merged 9 commits into from Mar 6, 2021

Conversation

JPery
Copy link
Contributor

@JPery JPery commented Dec 5, 2018

With this new feature we improve memory management. If the user only wanted a specific part of the audio file it had to import the whole audio file and then split it. With this new feature we only import the specific part of the file.

@JPery
Copy link
Contributor Author

JPery commented Dec 5, 2018

Note that this can't be made using current "parameters" variable because this params have to be before the output file name.

@JPery
Copy link
Contributor Author

JPery commented Dec 5, 2018

This second commit its due to this post.

@jiaaro
Copy link
Owner

jiaaro commented Jun 1, 2019

This looks promising! Sorry for the delay in responding, this needs a few things to be ready to merge:

  • Docs (API.markdown) need to be updated to reflect this change
  • This option should be exercised in the unit tests
  • Please add yourself to the AUTHORS file :)

@JPery
Copy link
Contributor Author

JPery commented Jun 2, 2019

I've added some tests in order to test the new features. This features weren't working in "wav" and "raw" files due to their import don't use ffmpeg, so I've cropped them after load (we can work on that in a future version).

@jiaaro
Copy link
Owner

jiaaro commented Jun 20, 2019

thanks for your work on this :) - it seems all of libav tests are still failing though, do you know why?

@JPery
Copy link
Contributor Author

JPery commented Jun 20, 2019

Based on the test logs it seems like libav is not skipping the start of the audio (in order to cut it) properly (segment is shorter than expected). You can see that in the Assertion Error:

Traceback (most recent call last):
  File "test/test.py", line 1255, in test_partial_load_start_second_equals_cropped_mp3_audio_segment
    self.assertEqual(len(partial_seg1), len(partial_seg2))
AssertionError: 9044 != 9036

The 'partial_seg1' var is shortened by indexing (AudioSegment.from_file(self.mp3_path_str)[1000:]) and the 'partial_seg2' var shortened by start_second param (AudioSegment.from_file(self.mp3_path_str, start_second=1.)[0:]).

I don't have libav installed so I couldn't test it properly.

@JPery
Copy link
Contributor Author

JPery commented Nov 2, 2019

Hi, @jiaaro.

I've been doing some research with this issue. It seems like it's a common issue with Libav. The library doesn't seek the audio properly (it seeks more frames than expected as commented in the docs too).

The feature is working fine with FFmpeg. I think it's ready to go. We can advise in the API that this feature isn't working as intended when using Libav, so people will know the issue.

Best regards,
Jorge

@JPery
Copy link
Contributor Author

JPery commented Oct 29, 2020

Hi, @jiaaro.

I've updated my repository with all the new changes you've introduced into the library. I've also seen that you removed the Libav dependency from Travis, who was the responsible for my test not passing Travis CI.

My PR should be compliant now, but Appveyor shows an error unrelated to the PR itself: "Start-FileDownloadInternal : Error downloading remote file: One or more errors occurred".

It seems that its unable to download the specified FFmpeg version.

I'm actively using my own branch as I need the feature I introduced, as it gives me significant performance improvements.

What can I do to fix the Appveyor build and merge the PR?

Thanks!

Updating to last pydub version
@JPery
Copy link
Contributor Author

JPery commented Jan 22, 2021

Hi, @jiaaro!

I've updated my code with the fix of Appveyor and all the test are passing as intended.

I think the PR is ready to go!

@JPery
Copy link
Contributor Author

JPery commented Feb 8, 2021

Hi!

Any plans on a release with this feature? It would be awesome for me as I'm using my custom repo as I need this feature in production.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants