Upgrades Angular-audio, Mutagen and MidiJs #6912

NishealJ · 2019-06-13T03:34:39Z

Explanation

This PR Upgrades Angular-audio, Mutagen and MidiJs.
for testing doc visit here

Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
The linter/Karma presubmit checks have passed.
- These should run automatically, but if not, you can manually trigger them locally using python scripts/pre_commit_linter.py and bash scripts/run_frontend_tests.sh.
The PR is made from a branch that's not called "develop".
The PR has an appropriate "CHANGELOG: ..." label (If you are unsure of which label to add, ask the reviewers for guidance).
The PR follows the style guide.
The PR addresses the points mentioned in the codeowner checks for the files/folders changed. (See the codeowner's wiki page.)
The PR is assigned to an appropriate reviewer.
- If you're a new contributor, please ask on Gitter for someone to assign a reviewer and don't tick this checkbox.
- If you're not sure who the appropriate reviewer is, please assign to the issue's "owner" -- see the "talk-to" label on the issue. Do not only request the review but also add the reviewer as an assignee.

NishealJ · 2019-06-13T03:47:01Z

Hi @DubeySandeep & @apb7,
here is the testing doc

codecov · 2019-06-13T06:40:36Z

Codecov Report

Merging #6912 into develop will decrease coverage by 0.18%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #6912      +/-   ##
===========================================
- Coverage     95.5%   95.32%   -0.19%     
===========================================
  Files          371      371              
  Lines        51058    50558     -500     
===========================================
- Hits         48762    48193     -569     
- Misses        2296     2365      +69

Impacted Files	Coverage Δ
appengine_config.py	`50% <ø> (ø)`	⬆️
core/tests/gae_suite.py	`74.41% <ø> (ø)`	⬆️
core/controllers/acl_decorators.py	`88.56% <0%> (-10.43%)`	⬇️
core/domain/topic_services.py	`92.44% <0%> (-1.14%)`	⬇️
core/domain/skill_services.py	`83.52% <0%> (-0.4%)`	⬇️
core/controllers/editor_test.py	`99.57% <0%> (-0.01%)`	⬇️
core/controllers/question_editor_test.py	`100% <0%> (ø)`	⬆️
core/controllers/resources_test.py	`100% <0%> (ø)`	⬆️
core/controllers/creator_dashboard_test.py	`100% <0%> (ø)`	⬆️
core/controllers/acl_decorators_test.py	`100% <0%> (ø)`	⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 742b1f8...0db9e10. Read the comment docs.

codecov · 2019-06-13T06:40:37Z

Codecov Report

Merging #6912 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #6912   +/-   ##
========================================
  Coverage    95.86%   95.86%           
========================================
  Files          371      371           
  Lines        52090    52090           
========================================
  Hits         49934    49934           
  Misses        2156     2156

Impacted Files	Coverage Δ
core/tests/gae_suite.py	`74.41% <ø> (ø)`	⬆️
appengine_config.py	`50% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1c2c00...695c14a. Read the comment docs.

apb7

Hi @NishealJ, just a minor comment. Left a comment on the testing doc as well. Thanks!
Also, @DubeySandeep, if you're around, would you like to take a pass on the PR and/or the test doc? Thanks!

apb7 · 2019-06-15T07:48:56Z

manifest.json

        "rootDirPrefix": "MIDI.js-",
-        "targetDir": "midi-js-2ef687"
+        "targetDirPrefix": "MIDI.js-"


Please use targetDir field specifying the commit, as in the previous pattern.

I don't think this has been addressed yet?

Hi @apb7, sorry missed has been addressed now. Thanks!

DubeySandeep · 2019-06-15T08:10:37Z

@NishealJ This PR needs testing on the following (which isn't documented in your test doc):
For mutagen:

Test uploading an audio file in a different format.
Test uploading audio at different bitRate (You can find audios here)
Test changing the extension of audio file (Like x.wav --> x.mp3).

For ng-audio:

Can play uploaded or recorded audio in the exploration player.
Can play uploaded or recorded audio in the translation tab.

For MIDI: (I think this is related to interaction)

Test this on MusicalNoteInteraction.

These are the overview of the places where you should test these upgrades. Also, you can ping (or assign) @mzaman07 for testing this PR as a team lead for the Voiceover and translation project.

apb7 · 2019-06-15T08:17:01Z

@DubeySandeep, thank you for these pointers. I am adding a link to this comment for reference in the testing doc. Thanks!

NishealJ · 2019-06-16T07:14:53Z

Hi @apb7 & @DubeySandeep,
I've updated the testing doc and i've tested for the following as suggested by @DubeySandeep
I've tested that the audio is uploaded recorded and played in all the views
I've tested that audio in different bitrates also provide good outcome and different file formats are handled correctly

For MidiJs, i,ve tested it with musical notes interaction and it works well

Thanks

apb7

@NishealJ, please address existing review comments before requesting another review. Thanks!

apb7 · 2019-06-16T15:06:12Z

manifest.json

        "rootDirPrefix": "MIDI.js-",
-        "targetDir": "midi-js-2ef687"
+        "targetDirPrefix": "MIDI.js-"


I don't think this has been addressed yet?

apb7

LGTM, @NishealJ! @DubeySandeep, can you please take a final pass (for the testing doc too)? Thanks!

apb7 · 2019-06-17T10:10:43Z

package-lock.json

@@ -1832,6 +1832,7 @@
        "anymatch": "^2.0.0",
        "async-each": "^1.0.1",
        "braces": "^2.3.2",
+        "fsevents": "^1.2.7",


@NishealJ, please revert these changes.

Done @apb7 Thanks!

DubeySandeep

@NishealJ, Thanks for the PR, LGTM!

Can you please check these two points as well:

Uploading mp3 audio of duration > 5 mins. fails in the backend.
Can you please check whether this issue is resolved in the new mutagen?

DubeySandeep · 2019-06-17T11:03:53Z

@vojtechjelinek, @seanlip & @nithusha21, Can you please review this PR as a code owner? Thanks! :)

NishealJ · 2019-06-17T11:46:50Z

this issue

Hi @DubeySandeep,

yes on uploading audio of duration > 5 min fails at the backend with the expected error.

This issues seems to be still there, to reproduce this I have saved "weloveoppia" as a .mp3 file hence making a false .mp3 On uploading this it throws mutagen.MutagenError which raises an error "Audio not recognized"

vojtechjelinek

LGTM from code owner's perspective. Thanks!

seanlip

Thanks -- LGTM as codeowner

seanlip · 2019-06-17T22:15:26Z

@nithusha21 PTAL as codeowner

nithusha21

LGTM as a codeowner

nithusha21 · 2019-06-18T06:20:07Z

Please note... Travis is failing...

oppiabot · 2019-06-19T10:47:47Z

Hi @NishealJ. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

NishealJ · 2019-06-21T06:00:13Z

Hi @apb7 & @seanlip, I Think this can go in Thanks!

kevinlee12

lgtm! (from code owner's)

NishealJ added 3 commits June 9, 2019 11:43

Upgrade Angular Audio and Mutagen

722f0c9

Merge

f024fc4

Add Midijs

0db9e10

NishealJ requested review from apb7, nithusha21, seanlip and vojtechjelinek as code owners June 13, 2019 03:34

apb7 reviewed Jun 15, 2019

View reviewed changes

apb7 assigned NishealJ Jun 15, 2019

NishealJ assigned DubeySandeep Jun 16, 2019

apb7 reviewed Jun 16, 2019

View reviewed changes

update target dir

044c01a

apb7 added the CHANGELOG: Code health label Jun 17, 2019

apb7 approved these changes Jun 17, 2019

View reviewed changes

apb7 unassigned NishealJ Jun 17, 2019

DubeySandeep approved these changes Jun 17, 2019

View reviewed changes

DubeySandeep assigned vojtechjelinek, seanlip and nithusha21 and unassigned DubeySandeep Jun 17, 2019

DubeySandeep assigned NishealJ Jun 17, 2019

NishealJ added 2 commits June 17, 2019 14:13

Revert fsevents

9442339

Revert More of fsevent

3bbd615

vojtechjelinek approved these changes Jun 17, 2019

View reviewed changes

vojtechjelinek removed their assignment Jun 17, 2019

Merge remote-tracking branch 'origin/develop' into a_audio_up

80efe30

seanlip approved these changes Jun 17, 2019

View reviewed changes

seanlip unassigned seanlip and NishealJ Jun 17, 2019

nithusha21 approved these changes Jun 18, 2019

View reviewed changes

nithusha21 removed their assignment Jun 18, 2019

Update dir

8128bf9

NishealJ requested a review from kevinlee12 as a code owner June 18, 2019 11:04

NishealJ added the WIP label Jun 19, 2019

Fix lint

bc618fb

NishealJ removed the WIP label Jun 19, 2019

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label Jun 19, 2019

NishealJ added 2 commits June 19, 2019 14:10

Reslove Merge conflicts

7a0f34d

Revert Package-lock.json

27b9dc6

NishealJ removed the PR: don't merge - HAS MERGE CONFLICTS label Jun 19, 2019

Merge remote-tracking branch 'origin/develop' into a_audio_up

695c14a

kevinlee12 approved these changes Jun 21, 2019

View reviewed changes

seanlip merged commit 780d1b1 into oppia:develop Jun 21, 2019

mzaman07 mentioned this pull request Mar 3, 2020

Error in console while playing audio for an exploration. #8637

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrades Angular-audio, Mutagen and MidiJs #6912

Upgrades Angular-audio, Mutagen and MidiJs #6912

NishealJ commented Jun 13, 2019 •

edited

Loading

NishealJ commented Jun 13, 2019

codecov bot commented Jun 13, 2019

codecov bot commented Jun 13, 2019 •

edited

Loading

apb7 left a comment

apb7 Jun 15, 2019

apb7 Jun 16, 2019

NishealJ Jun 17, 2019

DubeySandeep commented Jun 15, 2019

apb7 commented Jun 15, 2019

NishealJ commented Jun 16, 2019

apb7 left a comment

apb7 Jun 16, 2019

apb7 left a comment

apb7 Jun 17, 2019

NishealJ Jun 17, 2019

DubeySandeep left a comment

DubeySandeep commented Jun 17, 2019

NishealJ commented Jun 17, 2019

vojtechjelinek left a comment

seanlip left a comment

seanlip commented Jun 17, 2019

nithusha21 left a comment

nithusha21 commented Jun 18, 2019

oppiabot bot commented Jun 19, 2019

NishealJ commented Jun 21, 2019

kevinlee12 left a comment

Upgrades Angular-audio, Mutagen and MidiJs #6912

Upgrades Angular-audio, Mutagen and MidiJs #6912

Conversation

NishealJ commented Jun 13, 2019 • edited Loading

Explanation

Checklist

NishealJ commented Jun 13, 2019

codecov bot commented Jun 13, 2019

Codecov Report

codecov bot commented Jun 13, 2019 • edited Loading

Codecov Report

apb7 left a comment

Choose a reason for hiding this comment

apb7 Jun 15, 2019

Choose a reason for hiding this comment

apb7 Jun 16, 2019

Choose a reason for hiding this comment

NishealJ Jun 17, 2019

Choose a reason for hiding this comment

DubeySandeep commented Jun 15, 2019

apb7 commented Jun 15, 2019

NishealJ commented Jun 16, 2019

apb7 left a comment

Choose a reason for hiding this comment

apb7 Jun 16, 2019

Choose a reason for hiding this comment

apb7 left a comment

Choose a reason for hiding this comment

apb7 Jun 17, 2019

Choose a reason for hiding this comment

NishealJ Jun 17, 2019

Choose a reason for hiding this comment

DubeySandeep left a comment

Choose a reason for hiding this comment

DubeySandeep commented Jun 17, 2019

NishealJ commented Jun 17, 2019

vojtechjelinek left a comment

Choose a reason for hiding this comment

seanlip left a comment

Choose a reason for hiding this comment

seanlip commented Jun 17, 2019

nithusha21 left a comment

Choose a reason for hiding this comment

nithusha21 commented Jun 18, 2019

oppiabot bot commented Jun 19, 2019

NishealJ commented Jun 21, 2019

kevinlee12 left a comment

Choose a reason for hiding this comment

NishealJ commented Jun 13, 2019 •

edited

Loading

codecov bot commented Jun 13, 2019 •

edited

Loading