Ticket #4783 (closed defect: fixed)

Opened 6 years ago

Last modified 6 years ago

Browse can't play Video or Audio made by record

Reported by: AlexL Owned by: chihyu
Priority: blocker Milestone: Update.1
Component: sugar Version:
Keywords: review+, relnote Cc: AlexL, olpc-dev@…, tomeu, cscott, jg, ffm
Action Needed: Verified: yes
Deployments affected: Blocked By:
Blocking:

Description

build joyride 257 q2d04

There was a bug like this, which was fixed, but it has now reappeared. When you try to resume an audio or video file made by record, browse opens, and says it's downloading the file. After it completes, if you click open, it does nothing.

This may be related to bug http://dev.laptop.org/ticket/4782

Attachments

mime.patch (0.5 kB) - added by marco 6 years ago.
mime.2.patch (0.5 kB) - added by marco 6 years ago.
shell.log (8.8 kB) - added by chihyu 6 years ago.
rainbow (6.8 kB) - added by chihyu 6 years ago.
org.laptop.WebActivity-2.log (3.9 kB) - added by chihyu 6 years ago.
org.laptop.RecordActivity-1.log (1.2 kB) - added by chihyu 6 years ago.
messages (3.6 kB) - added by chihyu 6 years ago.
message recorded during the period when video was played by Browse
dmesg (25.7 kB) - added by chihyu 6 years ago.

Change History

follow-up: ↓ 3   Changed 6 years ago by jg

  • priority changed from high to blocker
  • milestone changed from Never Assigned to Update.1

Does this work if security turned off?

  Changed 6 years ago by erikb

  • owner changed from erikb to jg

Record-39.xo on Trial-3, b625, can successfully save photo+audio+video to the datastore and clipboard and also play those back successfully from the datastore and clipboard.

Thoughts: - Has something changed in how the clipboard works since then? What?

- Maybe audio and video files don't work because of the audio bug on joyride? (Although they can be played back in Record, so I am not sure how this would be effecting things).

Suggestions: - Would an activity that just adds a video or audio to the clipboard & datastore be helpful for debugging?

in reply to: ↑ 1   Changed 6 years ago by erikb

Replying to jg:

Does this work if security turned off?

Record does not launch with Security turned on.

  Changed 6 years ago by erikb

  • cc olpc-dev@… added

  Changed 6 years ago by tomeu

  • cc tomeu added
  • owner changed from jg to marco

The DS fails to add the correct extension to the checked-out file.

The following patch fixes the DS:

diff --git a/src/olpc/datastore/model.py b/src/olpc/datastore/model.py
index 4558a10..cdaee9f 100644
--- a/src/olpc/datastore/model.py
+++ b/src/olpc/datastore/model.py
@@ -14,7 +14,10 @@ import datetime
 import os
 import time
 import warnings
+import logging
+
 from sugar import mime
+
 from olpc.datastore.utils import timeparse
 
 
@@ -227,7 +230,17 @@ class Content(object):
                 else: v = ''
             d[k] = v
         return d
-    
+
+    def _get_extension_from_mimetype(self):
+        # try to get an extension from the mimetype if available
+        mt = self.get_property('mime_type', None)
+        if mt is not None:
+            ext = mime.get_primary_extension(mt)
+            # .ksh is a strange ext for plain text
+            if ext and ext == '.ksh': ext = '.txt'
+            if ext and ext == '.jpe': ext = '.jpg' # fixes #3163
+            return ext
+        return None
 
     def suggestName(self):
         # we look for certain known property names
@@ -237,6 +250,10 @@ class Content(object):
         # checkout name
         filename = self.get_property('filename', None)
         ext = self.get_property('ext', '')
+        if not ext:
+            ext = self._get_extension_from_mimetype()
+
+        logging.debug('Content.suggestName: %r %r' % (filename, ext))
 
         if filename:
             # some backingstores keep the full relative path
@@ -244,16 +261,9 @@ class Content(object):
             f, e = os.path.splitext(filename)
             if e: return filename, None
             if ext: return "%s.%s" % (filename, ext), None
-        elif ext: return None, ext
-        else:
-            # try to get an extension from the mimetype if available
-            mt = self.get_property('mime_type', None)
-            if mt:
-                ext = mime.get_primary_extension(mt)
-                # .ksh is a strange ext for plain text
-                if ext and ext == '.ksh': ext = '.txt'
-                if ext and ext == '.jpe': ext = '.jpg' # fixes #3163
-                if ext: return None, ext
+        elif ext:
+            return None, ext
+
         return None, None
 
     def get_file(self):

but that's not enough to make it work, as the globs files installed in the xo doesn't contain an entry for video/ogg.

If the patch is applied, an entry like below was added to the globs file and the plugin is configured to open the ogv extension, then it should work.

video/ogg:*.ogv

Passing to Marco to figure out the mimetypes issue.

  Changed 6 years ago by marco

  • owner changed from marco to tomeu

Back to Tomeu to get the DS patch in. We will have to discuss the mime issue to decide what to do for Update.1 and what later. audio/ogg and video/ogg are not currently supported by the system.

  Changed 6 years ago by tomeu

  • owner changed from tomeu to ApprovalForUpdate

Asking for permission to push that patch.

  Changed 6 years ago by jg

  • owner changed from ApprovalForUpdate to cscott

approved.

  Changed 6 years ago by tomeu

  • owner changed from cscott to marco

Pushed. Marco, can you make an rpm and then assign to Dennis/Scott for tagging?

  Changed 6 years ago by marco

  • cc cscott, jg added
  • owner changed from marco to dgilmore

Dennis please push this rpm to Update.1:

sugar-datastore-0.2.2-0.38.20071112git.6d3d607ec7

  Changed 6 years ago by marco

When that's done please just reassign back to me, we still need to solve the specific mime type issue.

  Changed 6 years ago by marco

  • keywords review? added

Changed 6 years ago by marco

Changed 6 years ago by marco

  Changed 6 years ago by tomeu

  • keywords review+ added; review? removed

  Changed 6 years ago by marco

  • component changed from camera-activity to sugar

  Changed 6 years ago by marco

Checked in on master

  Changed 6 years ago by cscott

  • owner changed from dgilmore to cscott
  • status changed from new to assigned

I'm managing the update.1 repos. Please leave these bugs assigned to me unless I reassign to Dennis, thanks.

How does this interact with the recently reverted sugar-datastore-0.3? Does the new 0.2 rpm contain anything other than this patch? Has this package been tested in joyride?

  Changed 6 years ago by marco

The change is in sugar-base. The datastore use this API but there is no relation with the reverted 0.3. It's not built in joyride yet.

What's the status of joyride btw? It doesn't seem to be hourly building...

  Changed 6 years ago by marco

Oh I think you are confused... The rpm referenced in comment 10 went in joyride a while ago (and so it was tagged for Update.1). mime.2.patch, which is what the ticket is still open for, is not yet in a build.

  Changed 6 years ago by marco

  • owner changed from cscott to marco
  • status changed from assigned to new

The ticket should be assigned to me so that I actually get the patch into a joyride build and then I ask for approval...

  Changed 6 years ago by marco

  • owner changed from marco to ApprovalForUpdate

Seem to be working fine in 332

  Changed 6 years ago by jg

  • owner changed from ApprovalForUpdate to cscott

OK for Update.1

  Changed 6 years ago by marco

  • owner changed from cscott to marco

I need to build an rpm fo Update-1. Please always assign back to me after Approval, unless I mention the nvr of the rpm to push...

  Changed 6 years ago by marco

Fedora package cvs seem to have problems atm, I'll build it when it comes back.

  Changed 6 years ago by marco

  • owner changed from marco to cscott

Package built: sugar-base-0.1.1-1

  Changed 6 years ago by kimquirk

  • keywords review+, relnote added; review+ removed

  Changed 6 years ago by AlexL

  • status changed from new to closed
  • verified set
  • resolution set to fixed

verified as fixed in joyride 345

  Changed 6 years ago by marco

  • status changed from closed to reopened
  • resolution deleted

This should stay open so that cscott gets it into Update.1... Thanks for testing!

  Changed 6 years ago by marco

  • owner changed from cscott to dgilmore
  • status changed from reopened to new

Looks like Dennis is now managing Update.1

  Changed 6 years ago by marco

As a reminder, the package to get in here is sugar-base-0.1.1-1

  Changed 6 years ago by ffm

  • cc ffm added
  • component changed from sugar to browse-activity

  Changed 6 years ago by ffm

  • component changed from browse-activity to sugar

  Changed 6 years ago by marco

  • status changed from new to closed
  • resolution set to fixed

Verified fixed in 657

  Changed 6 years ago by chihyu

  • status changed from closed to reopened
  • resolution deleted

Tested on joyride 1537 with firmware Q2D08, and the same problem occurred.

Is the patch included in the joyride builds as well, or just in Update.1?

follow-up: ↓ 36   Changed 6 years ago by marco

Works fine for me in 1537. Can you provide the exact steps to reproduce and logs please? http://wiki.laptop.org/go/Attaching_Sugar_Logs_to_Tickets

  Changed 6 years ago by marco

  • owner changed from dgilmore to chihyu
  • status changed from reopened to new

in reply to: ↑ 34   Changed 6 years ago by chihyu

It now works for me in 1537, but I do find some warning messages in the log files. Please see the attachment.

Replying to marco:

Works fine for me in 1537. Can you provide the exact steps to reproduce and logs please? http://wiki.laptop.org/go/Attaching_Sugar_Logs_to_Tickets

Changed 6 years ago by chihyu

Changed 6 years ago by chihyu

Changed 6 years ago by chihyu

Changed 6 years ago by chihyu

Changed 6 years ago by chihyu

message recorded during the period when video was played by Browse

Changed 6 years ago by chihyu

  Changed 6 years ago by tomeu

Could you please give it some more testing and close it if things are working now? Thanks.

  Changed 6 years ago by marco

  • status changed from new to closed
  • resolution set to fixed

Let's close it. Please open a separate ticket if you see it again and attach logs. (The warnings in the logs you posted are "normal" according to Erik).

Note: See TracTickets for help on using tickets.