New ABC version number for Flash Player "Serrano"

RESOLVED FIXED in Q3 11 - Serrano

Status

defect
P2
normal
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: lhansen, Assigned: lhansen)

Tracking

unspecified
Q3 11 - Serrano
Dependency tree / graph
Bug Flags:
flashplayer-bug -

Details

(Whiteboard: has-patch)

Attachments

(1 attachment, 1 obsolete attachment)

14.10 KB, patch
edwsmith
: review+
Details | Diff | Splinter Review
It has been decided (Edwin, Lars, Steven, based on prior discussions with Chris Brichfort) to rev the ABC version number in sync with the Flash Player version number, starting with Flash Player "Serrano".  This revving will allow the introduction of new facilities in the ABC format every time we ship a new Flash Player version.

The ABC parser in Tamarin must be able to parse all versions.

It has further been decided to increment the major ABC number, and then choose a minor ABC number that is the same as the version number selected for SWFs in the Flash Player that we're releasing with.

Thus for Serrano the ABC version will be 47.11 - the current version is 46.16 so the major version number goes to 47; for Serrano I believe the SWF version will be 11.  The minor number can be adjusted before release anyway.

The work items for this bug is to:

- clean up and generalize the version number parsing and tables containing
  version numbers
- set appropriate bits in the ABC state for the given ABC blob, based on
  the version number
- introduce new facilities in the ABC that are under ABC version control
  (code for those facilities must all be filed as separate bugs, and will
  necessarily discuss impacts on verification)
Update: Serrano will be 12, Spicy is 11.
Posted patch Patch (obsolete) — Splinter Review
Preliminary/bare-bones (but credible?) support for ABC version 47<<16|12, not including any actual parser/verifier/interpreter/code generator changes.
Attachment #498591 - Flags: review?(edwsmith)
Whiteboard: has-patch
(Ignore the xcode project file change, I'll update my xcode now.)
Comment on attachment 498591 [details] [diff] [review]
Patch


Nothing surprising or harmful, but setting floatSupport=1 with no functionality
checking the flag means this:  If by some crazy chance we ship it this way, 
then we will have "burned" abc version 47.12 because we wouldn't be able to
guarantee no such content existed (any such content would expect 47.12 to work
just like 46.16).

We probably don't care, just like we dont care that a particular abc version that never worked before (causing observable parsing exceptions) suddenly starts to work.

nit: various one-line comments with no . at the ends.
Attachment #498591 - Flags: review?(edwsmith) → review+
(In reply to comment #4)
> Comment on attachment 498591 [details] [diff] [review]
> Patch
> 
> 
> Nothing surprising or harmful, but setting floatSupport=1 with no functionality
> checking the flag means this:  If by some crazy chance we ship it this way, 
> then we will have "burned" abc version 47.12 because we wouldn't be able to
> guarantee no such content existed (any such content would expect 47.12 to work
> just like 46.16).

I don't understand that comment, can you please elaborate.

> nit: various one-line comments with no . at the ends.

Old habits die hard.  Will attempt to fix when I land.
Edwin:

Perhaps we need something like these:

  AVMFEATURE_SWF12
  AVMFEATURE_SWF13

The host software would enable the appropriate feature.  That way, if the host software does not support SWF13 it gets a chance to say that the AVM should not support the corresponding ABC format either.  This decouples AVM development from host software development.

We could also have

  AVMFEATURE_FLOAT
  AVMFEATURE_FLOAT4

probably, to control the setting of floatSupport; over time it might be phased out.  Not sure - this seems like an internal thing, not something that needs to be exposed necessarily.  Though might be useful for the host software to make use of the #define on the C++ level.
IM discussion with Edwin:

We'll add AVMFEATURE_SWF12, etc.  These will define VMCFG_SWF12, and so on.  But they may also define other names.  Specifically, AVMFEATURE_SWF12 will define VMCFG_FLOAT, VMCFG_FLOAT4, and so on, *if we land the float, float4 code and wish to enable it*.  External code (Flash Player) can test VMCFG_FLOAT, etc to see if the APIs are enabled, if they need that.  Internal code for the feature itself should be #ifdef VMCFG_FLOAT, etc.  (That's tricky for AS3 APIs but we'll figure it out if necessary.)
Posted patch Patch, v2Splinter Review
Supersedes the older patch and introduces the feature system.  Predefines features through SWF20, for illustrative purposes.  Changes to avmshell-features.h turn on SWF12 support now.
Attachment #498591 - Attachment is obsolete: true
Attachment #503114 - Flags: review?(edwsmith)
Comment on attachment 503114 [details] [diff] [review]
Patch, v2

Looks great.
Attachment #503114 - Flags: review?(edwsmith) → review+
Virgil points out that AVMFEATURE_SWF13 should depend on AVMFEATURE_SWF12, and so on.
Virgil also points out that using the #ifdef VMCFG_FLOAT etc internally is going to be a mess, and he has a point.  We should restrict this to be used by embedding code probably.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
changeset: 5801:96be8bbe9c8d
user:      Lars T Hansen <lhansen@adobe.com>
summary:   Fix 619926 - New ABC version number for Flash Player "Serrano" (r=edwsmith)

http://hg.mozilla.org/tamarin-redux/rev/96be8bbe9c8d
Flags: flashplayer-bug-
regarding http://hg.mozilla.org/tamarin-redux/rev/96be8bbe9c8d

3.17 +     *  47<<16|12   Content recognized by Flash Player "Serrano" and earlier (SWF <= 12)

serrano is now swf13 rather than 12.
You need to log in before you can comment on or make changes to this bug.