Closed Bug 953402 Opened 8 years ago Closed 8 years ago

[hamachi] Video file is incorrectly shown as .jpg in Gallery app

Categories

(Firefox OS Graveyard :: Gaia::Gallery, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ecui, Assigned: gnaneswar.devuni)

Details

Attachments

(1 file, 2 obsolete files)

46 bytes, text/x-github-pull-request
johnhu
: review+
Details | Review
Description:
When user check detailed info of a recorded video in Gallery app, the extension name of video file is incorrectly shown as .jpg. 

* Steps to reproduce
1) Record a video ( which is 3gpp by default) in Camera app. 
2) Go to Gallery app, find the video and see the detailed info of the video.

* Actual Result
The name of the 3gpp video is VID_xxxx.jpg.

* Expected Result
The name of the 3gpp video is VID_xxxx.3gp.

* Occurrence rate
100%

* Build Number
hamachi/1.3.0/nightly
Gaia:     2013-12-16 03:59:56  1752e9e8                         
BuildID   20131216004002                                                   
Version   28.0a2
It seems that in below function in gallery/js/info.js,  filename is coming as .jpg instead of .3gp.

replacing the name locally may solve the problem.

function populateMediaInfo(fileinfo) {
    var data = {
      'info-name': getFileName(fileinfo.name),
 ....
}
Attached patch Patch v1 (obsolete) — Splinter Review
I have identified fix for this bug, attaching the patch to the bug. The patch seems to be fixing the problem. It would replace file name from .jpg to .3gp for video file in populateMediaInfo().

However There is another ideal way of fixing the bug. I am not sure what is desired.

Alternate fix:
In existing code, name of the video file is setting with .jpg extension and same filename is being passed to  populateMediaInfo() function via showFileInformation().
Actually, the better fix would be changing the code which is setting the video file name as .jpg.

But it looks like few places, video file is being checked with extension as .jpg. So those changes would also need to be modified accordingly.
Attachment #8355501 - Flags: review?(johu)
Comment on attachment 8355501 [details] [diff] [review]
Patch v1

Review of attachment 8355501 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks Gnaneswar,

This patch looks good. I give this patch r- because it also loads wrong size for video file, we need to use req.result.size instead of fileinfo.size. We use the jpg file which is created by camera app to detect a video file. The fileinfo object is the information of this jpg file instead the real video file. That's the reason that we need to use req.result to show the correct information. And IMHO,

1. Since we already know that the fileinfo is a video in showFileInformation, we may play the trick there instead of populateMediaInfo.
2. We may use req.result to create a temporary object by coping name, type, size, lastModifiedDate (we use date in fileinfo). And we may link the metadata field from original fileinfo to the this temporary object and supply it as the argument for populateMediaInfo function.


BTW, we use github's pull request to do the review and merge the code. If it is possible, please create a pull request the the next patch. Thanks.
Attachment #8355501 - Flags: review?(johu) → review-
Thanks for the review comments.

>>it also loads wrong size for video file, we need to use req.result.size instead of fileinfo.size.
This change is already available existing code and it seems correct size is being shown for video file.


Attached updated patch in in which actual file name is set in showFileInformation function.
And I have used req.result to set the filename.

Github PR: https://github.com/mozilla-b2g/gaia/pull/15043

Please let me know your comments.
Attached patch Patch v2 (obsolete) — Splinter Review
Updated patch
Attachment #8355501 - Attachment is obsolete: true
Attached file Patch v3
John,
As discussed over IRC, attached updated Github PR.

Thanks for the comments!
Attachment #8356093 - Attachment is obsolete: true
Attachment #8357060 - Flags: review?(johu)
Comment on attachment 8357060 [details] [review]
Patch v3

Thanks for this patch. This version is better than before. Nice job.
Attachment #8357060 - Flags: review?(johu) → review+
merged to master:
https://github.com/mozilla-b2g/gaia/commit/cd8a439bedf516cbfb91eec1c97225e5130e2301
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Assignee: nobody → gnaneswar.devuni
You need to log in before you can comment on or make changes to this bug.